Re: [PATCH 07/21] qemu_hotplug: refactor qemuDomainDetachDiskLive and qemuDomainDetachDiskDevice

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/22/19 5:07 AM, Peter Krempa wrote:
On Thu, Mar 21, 2019 at 18:28:47 -0400, Laine Stump wrote:
qemuDomainDetachDiskDevice() is only called from one place. Moving the
contents of the function to that place makes
qemuDomainDetachDiskLive() more similar to the other Detach functions
called by the toplevel qemuDomainDetachDevice().

The goal is to make each of the device-type-specific functions do this:

   1) find the exact device
   2) do any device-specific validation
   3) do general validation
   4) do device-specific shutdown (only needed for net devices)
   5) do the common block of code to send device_del to qemu, then
      optionally wait for a corresponding DEVICE_DELETED event from
      qemu.

with the final aim being that only items 1 & 2 will remain in each
device-type-specific function, while 3 & 5 (which are the same for
almost every type) will be de-duplicated and moved to the toplevel
function that calls all of these (qemuDomainDetachDeviceLive(), which
will also contain a callout to the one instance of (4) (netdev).
I'm not sure whether this is worth setting into ston^w commit message.


Yeah, that should go below the --, but I can never remember to do that when sending the patch, so I just include the comments in the commit message and remove it later. (Is there a way to add "meta comments"?)


Signed-off-by: Laine Stump <laine@xxxxxxxxx>
---
  src/qemu/qemu_hotplug.c | 96 ++++++++++++++++++-----------------------
  1 file changed, 43 insertions(+), 53 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index af99f3bf4c..820929b109 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
[...]

@@ -5374,16 +5334,46 @@ qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver,
      case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
          virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                         _("disk device type '%s' cannot be detached"),
-                       virDomainDiskDeviceTypeToString(disk->device));
-        break;
+                       virDomainDiskDeviceTypeToString(detach->device));
+        return -1;
case VIR_DOMAIN_DISK_DEVICE_LAST:
      default:
-        virReportEnumRangeError(virDomainDiskDevice, disk->device);
+        virReportEnumRangeError(virDomainDiskDevice, detach->device);
          break;
Missing conversion to 'return -1;'


Ooh!! Good eye!!



ACK with the above fixed.


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux