[libvirt] [PATCH 0/2] virsh: Fix mishandling of inactive configuration on device hotplug
Peter Krempa (2): virsh: Don't use legacy API if --current is used on device hot(un)plug virsh: Use inactive definition when removing disk from config tools/virsh-domain.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) -- 1.8.5.2
https://bugzilla.redhat.com/show_bug.cgi?id=1049529 The legacy virDomainAttachDevice and virDomainDetachDevice operate only on active domains. When a user specified --current flag with an inactive domain the old API was used and reported an error. Fix it by calling the new API if --current is specified explicitly. --- tools/virsh-domain.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 3aabd26..5468365 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -230,7 +230,7 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (flags) + if (flags || current) rv = virDomainAttachDeviceFlags(dom, buffer, flags); else rv = virDomainAttachDevice(dom, buffer); @@ -669,7 +669,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) virDomainIsActive(dom) == 1) flags |= VIR_DOMAIN_AFFECT_LIVE; - if (flags) + if (flags || current) ret = virDomainAttachDeviceFlags(dom, xml, flags); else ret = virDomainAttachDevice(dom, xml); @@ -923,7 +923,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) xml = virBufferContentAndReset(&buf); - if (flags) + if (flags || current) ret = virDomainAttachDeviceFlags(dom, xml, flags); else ret = virDomainAttachDevice(dom, xml); @@ -9609,7 +9609,7 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (flags != 0) + if (flags != 0 || current) ret = virDomainDetachDeviceFlags(dom, buffer, flags); else ret = virDomainDetachDevice(dom, buffer); @@ -9884,7 +9884,7 @@ hit: goto cleanup; } - if (flags != 0) + if (flags != 0 || current) ret = virDomainDetachDeviceFlags(dom, detach_xml, flags); else ret = virDomainDetachDevice(dom, detach_xml); @@ -10189,7 +10189,7 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) VSH_PREPARE_DISK_XML_NONE))) goto cleanup; - if (flags != 0) + if (flags != 0 || current) ret = virDomainDetachDeviceFlags(dom, disk_xml, flags); else ret = virDomainDetachDevice(dom, disk_xml); -- 1.8.5.2
On 01/07/2014 10:12 AM, Peter Krempa wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1049529
The legacy virDomainAttachDevice and virDomainDetachDevice operate only on active domains. When a user specified --current flag with an inactive domain the old API was used and reported an error. Fix it by calling the new API if --current is specified explicitly. --- tools/virsh-domain.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
https://bugzilla.redhat.com/show_bug.cgi?id=1049529 The 'detach-disk' command in virsh used the active XML definition of a domain even when attempting to remove a disk from the config only. If the disk was only in the inactive definition the operation failed. Fix this by using the inactive XML in case that only the config is affected. --- tools/virsh-domain.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 5468365..506baf4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10175,7 +10175,12 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0) goto cleanup; - if (!(doc = virDomainGetXMLDesc(dom, 0))) + if (flags == VIR_DOMAIN_AFFECT_CONFIG) + doc = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE); + else + doc = virDomainGetXMLDesc(dom, 0); + + if (!doc) goto cleanup; if (persistent && -- 1.8.5.2
On 01/07/2014 10:12 AM, Peter Krempa wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1049529
The 'detach-disk' command in virsh used the active XML definition of a domain even when attempting to remove a disk from the config only. If the disk was only in the inactive definition the operation failed. Fix this by using the inactive XML in case that only the config is affected. --- tools/virsh-domain.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
ACK.
- if (!(doc = virDomainGetXMLDesc(dom, 0))) + if (flags == VIR_DOMAIN_AFFECT_CONFIG)
Might be a bit more robust as: (flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) == VIR_DOMAIN_AFFECT_CONFIG in case we have other flags set for other reasons; but right now we don't so this works. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/07/14 18:47, Eric Blake wrote:
On 01/07/2014 10:12 AM, Peter Krempa wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1049529
The 'detach-disk' command in virsh used the active XML definition of a domain even when attempting to remove a disk from the config only. If the disk was only in the inactive definition the operation failed. Fix this by using the inactive XML in case that only the config is affected. --- tools/virsh-domain.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
ACK.
- if (!(doc = virDomainGetXMLDesc(dom, 0))) + if (flags == VIR_DOMAIN_AFFECT_CONFIG)
Might be a bit more robust as: (flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) == VIR_DOMAIN_AFFECT_CONFIG in case we have other flags set for other reasons; but right now we don't so this works.
Thanks; Series pushed. Peter
participants (2)
-
Eric Blake -
Peter Krempa