When detaching a device it can be uniquely identified by its
alias. Instead of misusing virDomainDetachDeviceFlags which has
the same signature introduce new function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
include/libvirt/libvirt-domain.h | 3 +++
src/driver-hypervisor.h | 6 ++++++
src/libvirt-domain.c | 46 ++++++++++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 5 +++++
4 files changed, 60 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index d7cbd18796..da773b76cb 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2022,6 +2022,9 @@ int virDomainDetachDeviceFlags(virDomainPtr domain,
int virDomainUpdateDeviceFlags(virDomainPtr domain,
const char *xml, unsigned int flags);
+int virDomainDetachDeviceAlias(virDomainPtr domain,
+ const char *alias, unsigned int flags);
+
typedef struct _virDomainStatsRecord virDomainStatsRecord;
typedef virDomainStatsRecord *virDomainStatsRecordPtr;
struct _virDomainStatsRecord {
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index e71a72a441..9a224998fd 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -441,6 +441,11 @@ typedef int
const char *xml,
unsigned int flags);
+typedef int
+(*virDrvDomainDetachDeviceAlias)(virDomainPtr domain,
+ const char *alias,
+ unsigned int flags);
+
typedef int
(*virDrvDomainGetAutostart)(virDomainPtr domain,
int *autostart);
@@ -1392,6 +1397,7 @@ struct _virHypervisorDriver {
virDrvDomainDetachDevice domainDetachDevice;
virDrvDomainDetachDeviceFlags domainDetachDeviceFlags;
virDrvDomainUpdateDeviceFlags domainUpdateDeviceFlags;
+ virDrvDomainDetachDeviceAlias domainDetachDeviceAlias;
virDrvDomainGetAutostart domainGetAutostart;
virDrvDomainSetAutostart domainSetAutostart;
virDrvDomainGetSchedulerType domainGetSchedulerType;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 2d86e48979..b32c1d3064 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -8349,6 +8349,52 @@ virDomainUpdateDeviceFlags(virDomainPtr domain,
}
+/**
+ * virDomainDetachDeviceAlias:
+ * @domain: pointer to domain object
+ * @alias: device alias
+ * @flags: bitwise-OR of virDomainDeviceModifyFlags
+ *
+ * Detach a virtual device from a domain, using the alias to
+ * specify device.
+ *
+ * See virDomainDetachDeviceFlags() for more details.
+ *
+ * Returns 0 in case of success, -1 in case of failure.
+ */
+int
+virDomainDetachDeviceAlias(virDomainPtr domain,
+ const char *alias,
+ unsigned int flags)
+{
+ virConnectPtr conn;
+
+ VIR_DOMAIN_DEBUG(domain, "alias=%s, flags=0x%x", alias, flags);
+
+ virResetLastError();
+
+ virCheckDomainReturn(domain, -1);
+ conn = domain->conn;
+
+ virCheckNonNullArgGoto(alias, error);
+ virCheckReadOnlyGoto(conn->flags, error);
+
+ if (conn->driver->domainDetachDeviceAlias) {
+ int ret;
+ ret = conn->driver->domainDetachDeviceAlias(domain, alias, flags);
+ if (ret < 0)
+ goto error;
+ return ret;
+ }
+
+ virReportUnsupportedError();
+
+ error:
+ virDispatchError(domain->conn);
+ return -1;
+}
+
+
/**
* virConnectDomainEventRegister:
* @conn: pointer to the connection
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 95df3a0dbc..cd6b0c1fdc 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -785,4 +785,9 @@ LIBVIRT_4.1.0 {
virStoragePoolLookupByTargetPath;
} LIBVIRT_3.9.0;
+LIBVIRT_4.4.0 {
+ global:
+ virDomainDetachDeviceAlias;
+} LIBVIRT_4.1.0;
+
# .... define new API here using predicted next version number ....
--
2.16.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, May 21, 2018 at 18:07:58 +0200, Michal Privoznik wrote:
> When detaching a device it can be uniquely identified by its
> alias. Instead of misusing virDomainDetachDeviceFlags which has
> the same signature introduce new function.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> include/libvirt/libvirt-domain.h | 3 +++
> src/driver-hypervisor.h | 6 ++++++
> src/libvirt-domain.c | 46 ++++++++++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 5 +++++
> 4 files changed, 60 insertions(+)
[...]
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 2d86e48979..b32c1d3064 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -8349,6 +8349,52 @@ virDomainUpdateDeviceFlags(virDomainPtr domain,
> }
>
>
> +/**
> + * virDomainDetachDeviceAlias:
> + * @domain: pointer to domain object
> + * @alias: device alias
> + * @flags: bitwise-OR of virDomainDeviceModifyFlags
> + *
> + * Detach a virtual device from a domain, using the alias to
> + * specify device.
> + *
> + * See virDomainDetachDeviceFlags() for more details.
Since this is a new API I think that we should finally fix the broken
semantics of the old API and return a 'timeout' code rather than
success. Obviously this will make the implementation quite more
challenging, but the old semantics are really broken.
Without this it is just syntax-sugar for callers which are too lazy to
extract the snippet from the XML and pass it to the old API.
> + *
> + * Returns 0 in case of success, -1 in case of failure.
> + */
> +int
> +virDomainDetachDeviceAlias(virDomainPtr domain,
> + const char *alias,
> + unsigned int flags)
> +{
> + virConnectPtr conn;
> +
> + VIR_DOMAIN_DEBUG(domain, "alias=%s, flags=0x%x", alias, flags);
> +
> + virResetLastError();
> +
> + virCheckDomainReturn(domain, -1);
> + conn = domain->conn;
> +
> + virCheckNonNullArgGoto(alias, error);
> + virCheckReadOnlyGoto(conn->flags, error);
> +
> + if (conn->driver->domainDetachDeviceAlias) {
> + int ret;
> + ret = conn->driver->domainDetachDeviceAlias(domain, alias, flags);
> + if (ret < 0)
> + goto error;
> + return ret;
> + }
> +
> + virReportUnsupportedError();
> +
> + error:
> + virDispatchError(domain->conn);
> + return -1;
> +}
> +
> +
> /**
> * virConnectDomainEventRegister:
> * @conn: pointer to the connection
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 95df3a0dbc..cd6b0c1fdc 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -785,4 +785,9 @@ LIBVIRT_4.1.0 {
> virStoragePoolLookupByTargetPath;
> } LIBVIRT_3.9.0;
>
> +LIBVIRT_4.4.0 {
> + global:
> + virDomainDetachDeviceAlias;
> +} LIBVIRT_4.1.0;
> +
> # .... define new API here using predicted next version number ....
> --
> 2.16.1
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 05/22/2018 08:36 AM, Peter Krempa wrote: > On Mon, May 21, 2018 at 18:07:58 +0200, Michal Privoznik wrote: >> When detaching a device it can be uniquely identified by its >> alias. Instead of misusing virDomainDetachDeviceFlags which has >> the same signature introduce new function. >> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >> --- >> include/libvirt/libvirt-domain.h | 3 +++ >> src/driver-hypervisor.h | 6 ++++++ >> src/libvirt-domain.c | 46 ++++++++++++++++++++++++++++++++++++++++ >> src/libvirt_public.syms | 5 +++++ >> 4 files changed, 60 insertions(+) > > [...] > >> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c >> index 2d86e48979..b32c1d3064 100644 >> --- a/src/libvirt-domain.c >> +++ b/src/libvirt-domain.c >> @@ -8349,6 +8349,52 @@ virDomainUpdateDeviceFlags(virDomainPtr domain, >> } >> >> >> +/** >> + * virDomainDetachDeviceAlias: >> + * @domain: pointer to domain object >> + * @alias: device alias >> + * @flags: bitwise-OR of virDomainDeviceModifyFlags >> + * >> + * Detach a virtual device from a domain, using the alias to >> + * specify device. >> + * >> + * See virDomainDetachDeviceFlags() for more details. > > Since this is a new API I think that we should finally fix the broken > semantics of the old API and return a 'timeout' code rather than > success. Obviously this will make the implementation quite more > challenging, but the old semantics are really broken. So just to make sure I understand. We would still wait for some time (just like we are doing now with virDomainDetachDeviceFlags()), but if the qemu event does not arrive in defined timeout, an error is reported (whereas virDomainDetachDeviceFlags() returns success). Well couple of points to make there: 1) I bet there will be somebody wanting us to make the timeout configurable (bad idea if you ask me), 2) users/mgmt apps still need to implement DEVICE_DELETED event handling. If it is so, why not make this completely asynchronous and basically return true/false - "yes we did send request to qemu"? That way we don't have to care about any timeouts, and this still could be a mere drop in replacement for virDomainDetachDevice(). Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, May 22, 2018 at 13:28:45 +0200, Michal Privoznik wrote: > On 05/22/2018 08:36 AM, Peter Krempa wrote: > > On Mon, May 21, 2018 at 18:07:58 +0200, Michal Privoznik wrote: > >> When detaching a device it can be uniquely identified by its > >> alias. Instead of misusing virDomainDetachDeviceFlags which has > >> the same signature introduce new function. > >> > >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > >> --- > >> include/libvirt/libvirt-domain.h | 3 +++ > >> src/driver-hypervisor.h | 6 ++++++ > >> src/libvirt-domain.c | 46 ++++++++++++++++++++++++++++++++++++++++ > >> src/libvirt_public.syms | 5 +++++ > >> 4 files changed, 60 insertions(+) > > > > [...] > > > >> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c > >> index 2d86e48979..b32c1d3064 100644 > >> --- a/src/libvirt-domain.c > >> +++ b/src/libvirt-domain.c > >> @@ -8349,6 +8349,52 @@ virDomainUpdateDeviceFlags(virDomainPtr domain, > >> } > >> > >> > >> +/** > >> + * virDomainDetachDeviceAlias: > >> + * @domain: pointer to domain object > >> + * @alias: device alias > >> + * @flags: bitwise-OR of virDomainDeviceModifyFlags > >> + * > >> + * Detach a virtual device from a domain, using the alias to > >> + * specify device. > >> + * > >> + * See virDomainDetachDeviceFlags() for more details. > > > > Since this is a new API I think that we should finally fix the broken > > semantics of the old API and return a 'timeout' code rather than > > success. Obviously this will make the implementation quite more > > challenging, but the old semantics are really broken. > > So just to make sure I understand. We would still wait for some time > (just like we are doing now with virDomainDetachDeviceFlags()), but if > the qemu event does not arrive in defined timeout, an error is reported > (whereas virDomainDetachDeviceFlags() returns success). > > Well couple of points to make there: > 1) I bet there will be somebody wanting us to make the timeout > configurable (bad idea if you ask me), > 2) users/mgmt apps still need to implement DEVICE_DELETED event handling. > > If it is so, why not make this completely asynchronous and basically > return true/false - "yes we did send request to qemu"? That way we don't > have to care about any timeouts, and this still could be a mere drop in > replacement for virDomainDetachDevice(). That is fine with me. It even simplifies the detachment code since it doesn't have to register the handler to call back for synchronous release. On the other hand ... it probably will require new detachment code. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.