[libvirt] [PATCH 0/5] Introduce virDomainDetachDeviceAlias
The idea is to allow user to detach device passing only alias. It is enough to uniquely identify a device. Michal Privoznik (5): Introduce virDomainDetachDeviceAlias API remote: Implement virDomainDetachDeviceAlias qemu: Split qemuDomainDetachDeviceLiveAndConfig qemu: Implement virDomainDetachDeviceAlias virsh: Expose virDomainDetachDeviceAlias include/libvirt/libvirt-domain.h | 3 ++ src/driver-hypervisor.h | 6 +++ src/libvirt-domain.c | 46 +++++++++++++++++++++ src/libvirt_public.syms | 5 +++ src/qemu/qemu_driver.c | 87 +++++++++++++++++++++++++++++++++++----- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 16 +++++++- src/remote_protocol-structs | 6 +++ tools/virsh-domain.c | 79 ++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 15 +++++++ 10 files changed, 253 insertions(+), 11 deletions(-) -- 2.16.1
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
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
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
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.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 16 +++++++++++++++- src/remote_protocol-structs | 6 ++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 95437b4365..8695046b71 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8308,6 +8308,7 @@ static virHypervisorDriver hypervisor_driver = { .domainDetachDevice = remoteDomainDetachDevice, /* 0.3.0 */ .domainDetachDeviceFlags = remoteDomainDetachDeviceFlags, /* 0.7.7 */ .domainUpdateDeviceFlags = remoteDomainUpdateDeviceFlags, /* 0.8.0 */ + .domainDetachDeviceAlias = remoteDomainDetachDeviceAlias, /* 4.4.0 */ .domainGetAutostart = remoteDomainGetAutostart, /* 0.3.0 */ .domainSetAutostart = remoteDomainSetAutostart, /* 0.3.0 */ .domainGetSchedulerType = remoteDomainGetSchedulerType, /* 0.3.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 296a087181..b23c93514a 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1284,6 +1284,12 @@ struct remote_domain_update_device_flags_args { unsigned int flags; }; +struct remote_domain_detach_device_alias_args { + remote_nonnull_domain dom; + remote_nonnull_string alias; + unsigned int flags; +}; + struct remote_domain_get_autostart_args { remote_nonnull_domain dom; }; @@ -6135,5 +6141,13 @@ enum remote_procedure { * @priority: high * @acl: storage_pool:getattr */ - REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391 + REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391, + + /** + * @generate: both + * @acl: domain:write + * @acl: domain:save:!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE + * @acl: domain:save:VIR_DOMAIN_AFFECT_CONFIG + */ + REMOTE_PROC_DOMAIN_DETACH_DEVICE_ALIAS = 392 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index fe163db73f..1b4fbc5d4b 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -915,6 +915,11 @@ struct remote_domain_update_device_flags_args { remote_nonnull_string xml; u_int flags; }; +struct remote_domain_detach_device_alias_args { + remote_nonnull_domain dom; + remote_nonnull_string alias; + u_int flags; +}; struct remote_domain_get_autostart_args { remote_nonnull_domain dom; }; @@ -3269,4 +3274,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MANAGED_SAVE_DEFINE_XML = 389, REMOTE_PROC_DOMAIN_SET_LIFECYCLE_ACTION = 390, REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391, + REMOTE_PROC_DOMAIN_DETACH_DEVICE_ALIAS = 392, }; -- 2.16.1
The aim is to have qemuDomainDetachDeviceLiveAndConfig() accept virDomainDeviceDefPtr. Therefore, new qemuDomainDetachDeviceXMLLiveAndConfig() which is a drop in replacement for the former. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 46 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3a328e5d46..0e0c0ce53e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8661,15 +8661,16 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, return ret; } + static int qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, virDomainObjPtr vm, - const char *xml, + virDomainDeviceDefPtr dev, unsigned int flags) { virCapsPtr caps = NULL; virQEMUDriverConfigPtr cfg = NULL; - virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; + virDomainDeviceDefPtr dev_copy = dev; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; virDomainDefPtr vmdef = NULL; int ret = -1; @@ -8686,12 +8687,6 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, !(flags & VIR_DOMAIN_AFFECT_LIVE)) parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - parse_flags); - if (dev == NULL) - goto cleanup; - if (flags & VIR_DOMAIN_AFFECT_CONFIG && flags & VIR_DOMAIN_AFFECT_LIVE) { /* If we are affecting both CONFIG and LIVE @@ -8743,11 +8738,42 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, virObjectUnref(cfg); if (dev != dev_copy) virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); virDomainDefFree(vmdef); return ret; } + +static int +qemuDomainDetachDeviceXMLLiveAndConfig(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *xml, + unsigned int flags) +{ + virCapsPtr caps = NULL; + virDomainDeviceDefPtr dev = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; + int ret = -1; + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto cleanup; + + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + !(flags & VIR_DOMAIN_AFFECT_LIVE)) + parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; + + if (!(dev = virDomainDeviceDefParse(xml, vm->def, + caps, driver->xmlopt, + parse_flags))) + goto cleanup; + + ret = qemuDomainDetachDeviceLiveAndConfig(driver, vm, dev, flags); + cleanup: + virDomainDeviceDefFree(dev); + virObjectUnref(caps); + return ret; +} + + static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, @@ -8769,7 +8795,7 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom, if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; - if (qemuDomainDetachDeviceLiveAndConfig(driver, vm, xml, flags) < 0) + if (qemuDomainDetachDeviceXMLLiveAndConfig(driver, vm, xml, flags) < 0) goto endjob; ret = 0; -- 2.16.1
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0e0c0ce53e..fee152d202 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8808,6 +8808,46 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom, return ret; } + +static int +qemuDomainDetachDeviceAlias(virDomainPtr dom, + const char *alias, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + virDomainDeviceDef dev; + int ret = -1; + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainDetachDeviceAliasEnsureACL(dom->conn, vm->def, flags) < 0) + goto cleanup; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainDefFindDevice(vm->def, alias, &dev, true) < 0) + goto endjob; + + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) + goto endjob; + + if (qemuDomainDetachDeviceLiveAndConfig(driver, vm, &dev, flags) < 0) + goto endjob; + + ret = 0; + + endjob: + qemuDomainObjEndJob(driver, vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + + static int qemuDomainDetachDevice(virDomainPtr dom, const char *xml) { return qemuDomainDetachDeviceFlags(dom, xml, @@ -21303,6 +21343,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainDetachDevice = qemuDomainDetachDevice, /* 0.5.0 */ .domainDetachDeviceFlags = qemuDomainDetachDeviceFlags, /* 0.7.7 */ .domainUpdateDeviceFlags = qemuDomainUpdateDeviceFlags, /* 0.8.0 */ + .domainDetachDeviceAlias = qemuDomainDetachDeviceAlias, /* 4.4.0 */ .domainGetAutostart = qemuDomainGetAutostart, /* 0.2.1 */ .domainSetAutostart = qemuDomainSetAutostart, /* 0.2.1 */ .domainGetSchedulerType = qemuDomainGetSchedulerType, /* 0.7.0 */ -- 2.16.1
On Mon, May 21, 2018 at 18:08:01 +0200, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0e0c0ce53e..fee152d202 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8808,6 +8808,46 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom, return ret; }
+ +static int +qemuDomainDetachDeviceAlias(virDomainPtr dom, + const char *alias, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + virDomainDeviceDef dev; + int ret = -1; + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainDetachDeviceAliasEnsureACL(dom->conn, vm->def, flags) < 0) + goto cleanup; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainDefFindDevice(vm->def, alias, &dev, true) < 0) + goto endjob;
This reports an 'internal error' if the alias is not found which is not great to present to users. Additionally this does not deal with the active/inactive flag which is wrong since you e.g. can't properly use this if you have a different persistent configuration (which uses user-aliases). In that case you can't refer to a alias in that configuration, since only vm->def is inspected.
+ + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) + goto endjob; + + if (qemuDomainDetachDeviceLiveAndConfig(driver, vm, &dev, flags) < 0) + goto endjob;
Since you pass in 'dev' containing a device definition which gets freed by this function it may become tricky. At least you will need to document that this is happening in this function. Note that I did not inspect all the functions called by qemuDomainDetachDeviceLiveAndConfig to ensure that there isn't an actual problem.
+ + ret = 0; + + endjob: + qemuDomainObjEndJob(driver, vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + + static int qemuDomainDetachDevice(virDomainPtr dom, const char *xml) { return qemuDomainDetachDeviceFlags(dom, xml, @@ -21303,6 +21343,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainDetachDevice = qemuDomainDetachDevice, /* 0.5.0 */ .domainDetachDeviceFlags = qemuDomainDetachDeviceFlags, /* 0.7.7 */ .domainUpdateDeviceFlags = qemuDomainUpdateDeviceFlags, /* 0.8.0 */ + .domainDetachDeviceAlias = qemuDomainDetachDeviceAlias, /* 4.4.0 */ .domainGetAutostart = qemuDomainGetAutostart, /* 0.2.1 */ .domainSetAutostart = qemuDomainSetAutostart, /* 0.2.1 */ .domainGetSchedulerType = qemuDomainGetSchedulerType, /* 0.7.0 */ -- 2.16.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 15 ++++++++++ 2 files changed, 94 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cfbbf5a7bc..b41e76b3d8 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11878,6 +11878,79 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd) return funcRet; } + +/* + * "detach-device-alias" command + */ +static const vshCmdInfo info_detach_device_alias[] = { + {.name = "help", + .data = N_("detach device from an alias") + }, + {.name = "desc", + .data = N_("Detach device from domain using given alias to identify device") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_detach_device_alias[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "alias", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("device alias") + }, + VIRSH_COMMON_OPT_DOMAIN_PERSISTENT, + VIRSH_COMMON_OPT_DOMAIN_CONFIG, + VIRSH_COMMON_OPT_DOMAIN_LIVE, + VIRSH_COMMON_OPT_DOMAIN_CURRENT, + {.name = NULL} +}; + +static bool +cmdDetachDeviceAlias(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + const char *alias = NULL; + bool current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool persistent = vshCommandOptBool(cmd, "persistent"); + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + bool ret = false; + + VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current); + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config || persistent) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (persistent && + virDomainIsActive(dom) == 1) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0) + goto cleanup; + + if (virDomainDetachDeviceAlias(dom, alias, flags) < 0) { + vshError(ctl, _("Failed to detach device with alias %s"), alias); + goto cleanup; + } + + vshPrintExtra(ctl, "%s", _("Device detached successfully\n")); + ret = true; + + cleanup: + virshDomainFree(dom); + return ret; +} + + /* * "update-device" command */ @@ -13999,6 +14072,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_detach_device, .flags = 0 }, + {.name = "detach-device-alias", + .handler = cmdDetachDeviceAlias, + .opts = opts_detach_device_alias, + .info = info_detach_device_alias, + .flags = 0 + }, {.name = "detach-disk", .handler = cmdDetachDisk, .opts = opts_detach_disk, diff --git a/tools/virsh.pod b/tools/virsh.pod index 929958a953..b6e60ce2f7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3112,6 +3112,21 @@ an offline domain, and like I<--live> I<--config> for a running domain. Note that older versions of virsh used I<--config> as an alias for I<--persistent>. +=item B<detach-device-alias> I<domain> I<alias> +[[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] + +Detach a device with given I<alias> from the I<domain>. + +If I<--live> is specified, affect a running domain. +If I<--config> is specified, affect the next startup of a persistent domain. +If I<--current> is specified, affect the current domain state. +Both I<--live> and I<--config> flags may be given, but I<--current> is +exclusive. When no flag is specified legacy API is used whose behavior depends +on the hypervisor driver. + +For compatibility purposes, I<--persistent> behaves like I<--config> for +an offline domain, and like I<--live> I<--config> for a running domain. + =item B<detach-disk> I<domain> I<target> [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] [I<--print-xml>] -- 2.16.1
On Mon, May 21, 2018 at 18:08:02 +0200, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 15 ++++++++++ 2 files changed, 94 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cfbbf5a7bc..b41e76b3d8 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11878,6 +11878,79 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd) return funcRet; }
+ +/* + * "detach-device-alias" command + */ +static const vshCmdInfo info_detach_device_alias[] = { + {.name = "help", + .data = N_("detach device from an alias") + }, + {.name = "desc", + .data = N_("Detach device from domain using given alias to identify device") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_detach_device_alias[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "alias", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("device alias") + }, + VIRSH_COMMON_OPT_DOMAIN_PERSISTENT,
... see below
+ VIRSH_COMMON_OPT_DOMAIN_CONFIG, + VIRSH_COMMON_OPT_DOMAIN_LIVE, + VIRSH_COMMON_OPT_DOMAIN_CURRENT, + {.name = NULL} +}; + +static bool +cmdDetachDeviceAlias(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + const char *alias = NULL; + bool current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool persistent = vshCommandOptBool(cmd, "persistent"); + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + bool ret = false; + + VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current); + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config || persistent) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (persistent && + virDomainIsActive(dom) == 1) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0) + goto cleanup; + + if (virDomainDetachDeviceAlias(dom, alias, flags) < 0) { + vshError(ctl, _("Failed to detach device with alias %s"), alias); + goto cleanup; + } + + vshPrintExtra(ctl, "%s", _("Device detached successfully\n")); + ret = true; + + cleanup: + virshDomainFree(dom); + return ret; +} + + /* * "update-device" command */ @@ -13999,6 +14072,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_detach_device, .flags = 0 }, + {.name = "detach-device-alias", + .handler = cmdDetachDeviceAlias, + .opts = opts_detach_device_alias, + .info = info_detach_device_alias, + .flags = 0 + }, {.name = "detach-disk", .handler = cmdDetachDisk, .opts = opts_detach_disk, diff --git a/tools/virsh.pod b/tools/virsh.pod index 929958a953..b6e60ce2f7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3112,6 +3112,21 @@ an offline domain, and like I<--live> I<--config> for a running domain. Note that older versions of virsh used I<--config> as an alias for I<--persistent>.
+=item B<detach-device-alias> I<domain> I<alias> +[[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] + +Detach a device with given I<alias> from the I<domain>. + +If I<--live> is specified, affect a running domain. +If I<--config> is specified, affect the next startup of a persistent domain. +If I<--current> is specified, affect the current domain state. +Both I<--live> and I<--config> flags may be given, but I<--current> is +exclusive. When no flag is specified legacy API is used whose behavior depends +on the hypervisor driver. + +For compatibility purposes, I<--persistent> behaves like I<--config> for +an offline domain, and like I<--live> I<--config> for a running domain.
I don't think we should continue adding this to new APIs.
+ =item B<detach-disk> I<domain> I<target> [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] [I<--print-xml>] -- 2.16.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Michal Privoznik -
Peter Krempa