[libvirt] [PATCH 13/21] qemu_hotplug: move qemuDomainDetachDeviceLive() to qemu_hotplug.c

Laine Stump laine at laine.org
Fri Mar 22 13:36:31 UTC 2019


On 3/22/19 7:36 AM, Peter Krempa wrote:
> On Thu, Mar 21, 2019 at 18:28:53 -0400, Laine Stump wrote:
>> This function is going to take on some of the functionality of its
>> subordinate functions, which all live in qemu_hotplug.c.
>>
>> Signed-off-by: Laine Stump <laine at laine.org>
>> ---
>>   src/qemu/qemu_driver.c  |  95 -----------------------------
>>   src/qemu/qemu_hotplug.c | 129 +++++++++++++++++++++++++++++++++++-----
>>   src/qemu/qemu_hotplug.h |  68 ++++++---------------
>>   3 files changed, 133 insertions(+), 159 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index a16eab5467..0b80004c6e 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -8004,101 +8004,6 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
>>       return ret;
>>   }
>>   
>> -static int
>> -qemuDomainDetachDeviceControllerLive(virQEMUDriverPtr driver,
>> -                                     virDomainObjPtr vm,
>> -                                     virDomainDeviceDefPtr dev,
>> -                                     bool async)
>> -{
>> -    virDomainControllerDefPtr cont = dev->data.controller;
>> -    int ret = -1;
>> -
>> -    switch (cont->type) {
>> -    case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
>> -        ret = qemuDomainDetachControllerDevice(driver, vm, dev, async);
>> -        break;
>> -    default :
>> -        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
>> -                       _("'%s' controller cannot be hot unplugged."),
>> -                       virDomainControllerTypeToString(cont->type));
>> -    }
>> -    return ret;
>> -}
> This function is not just moved ...
>
> [...]
>
>>   static int
>>   qemuDomainChangeDiskLive(virDomainObjPtr vm,
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index 1f96f56942..c7aba74c6b 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -5540,10 +5540,11 @@ static bool qemuDomainControllerIsBusy(virDomainObjPtr vm,
>>       }
>>   }
>>   
>> -int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
>> -                                     virDomainObjPtr vm,
>> -                                     virDomainDeviceDefPtr dev,
>> -                                     bool async)
>> +static int
>> +qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
>> +                                 virDomainObjPtr vm,
>> +                                 virDomainDeviceDefPtr dev,
>> +                                 bool async)
> ... but replaced. This is a separate semantic change.


No, those are two different functions. In the original code, you had 
this call chain:


qemuDomainDetachDeviceLive
   qemuDomainDetachDeviceControllerLive
     qemuDomainDetachControllerDevice

and after the patch you have the same call chain. In a later patch, I merge qemuDomainDetachDeviceControllerLive and qemuDomainDeviceControllerDevice into a single function, but in this patch it's pure movement.

>
>>   {
>>       int idx, ret = -1;
>>       virDomainControllerDefPtr detach = NULL;
>> @@ -5594,10 +5595,11 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
>>   
>>   
>>   /* search for a hostdev matching dev and detach it */
>> -int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
>> -                               virDomainObjPtr vm,
>> -                               virDomainDeviceDefPtr dev,
>> -                               bool async)
>> +static int
>> +qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
>> +                           virDomainObjPtr vm,
>> +                           virDomainDeviceDefPtr dev,
>> +                           bool async)
>>   {
>>       virDomainHostdevDefPtr hostdev = dev->data.hostdev;
>>       virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys;
>> @@ -5821,7 +5823,7 @@ qemuDomainDetachWatchdog(virQEMUDriverPtr driver,
>>   }
>>   
>>   
>> -int
>> +static int
>>   qemuDomainDetachRedirdevDevice(virQEMUDriverPtr driver,
>>                                  virDomainObjPtr vm,
>>                                  virDomainRedirdevDefPtr dev,
>> @@ -5865,7 +5867,7 @@ qemuDomainDetachRedirdevDevice(virQEMUDriverPtr driver,
>>   }
>>   
>>   
>> -int
>> +static int
>>   qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
>>                             virDomainObjPtr vm,
>>                             virDomainDeviceDefPtr dev,
>> @@ -5988,7 +5990,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
>>   }
>>   
>>   
>> -int
>> +static int
>>   qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
>>                             virDomainObjPtr vm,
>>                             virDomainRNGDefPtr rng,
>> @@ -6034,7 +6036,7 @@ qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
>>   }
>>   
>>   
>> -int
>> +static int
>>   qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
>>                                virDomainObjPtr vm,
>>                                virDomainMemoryDefPtr memdef,
>> @@ -6082,7 +6084,7 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
>>   }
>>   
>>   
>> -int
>> +static int
>>   qemuDomainDetachInputDevice(virDomainObjPtr vm,
>>                               virDomainInputDefPtr def,
>>                               bool async)
>> @@ -6133,7 +6135,7 @@ qemuDomainDetachInputDevice(virDomainObjPtr vm,
>>   }
>>   
>>   
>> -int
>> +static int
>>   qemuDomainDetachVsockDevice(virDomainObjPtr vm,
>>                               virDomainVsockDefPtr dev,
>>                               bool async)
>> @@ -6169,7 +6171,7 @@ qemuDomainDetachVsockDevice(virDomainObjPtr vm,
>>   }
>>   
>>   
>> -int
>> +static int
>>   qemuDomainDetachLease(virQEMUDriverPtr driver,
>>                         virDomainObjPtr vm,
>>                         virDomainLeaseDefPtr lease)
> I'd also prefer if these changes are made separately.


You mean changing the functions from global to static. Sure, I can do that.


>
>> @@ -6193,6 +6195,103 @@ qemuDomainDetachLease(virQEMUDriverPtr driver,
>>   }
>>   
>>   
>> +static int
>> +qemuDomainDetachDeviceControllerLive(virQEMUDriverPtr driver,
>> +                                     virDomainObjPtr vm,
>> +                                     virDomainDeviceDefPtr dev,
>> +                                     bool async)
>> +{
>> +    virDomainControllerDefPtr cont = dev->data.controller;
>> +    int ret = -1;
>> +
>> +    switch (cont->type) {
>> +    case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
>> +        ret = qemuDomainDetachControllerDevice(driver, vm, dev, async);
>> +        break;
>> +    default :
>> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
>> +                       _("'%s' controller cannot be hot unplugged."),
>> +                       virDomainControllerTypeToString(cont->type));
>> +    }
>> +    return ret;
>> +}


^^^^^ This is the original qemuDomainDetachDeviceControllerLive(), moved 
intact from one file to the other.


>> +
>> +int
>> +qemuDomainDetachDeviceLive(virDomainObjPtr vm,
>> +                           virDomainDeviceDefPtr dev,
>> +                           virQEMUDriverPtr driver,
>> +                           bool async)
>> +{
>> +    int ret = -1;
>> +
>> +    switch ((virDomainDeviceType)dev->type) {
>> +    case VIR_DOMAIN_DEVICE_DISK:
>> +        ret = qemuDomainDetachDeviceDiskLive(driver, vm, dev, async);
>> +        break;
>> +    case VIR_DOMAIN_DEVICE_CONTROLLER:
>> +        ret = qemuDomainDetachDeviceControllerLive(driver, vm, dev, async);
>> +        break;
>> +    case VIR_DOMAIN_DEVICE_LEASE:
>> +        ret = qemuDomainDetachLease(driver, vm, dev->data.lease);
>> +        break;
>> +    case VIR_DOMAIN_DEVICE_NET:
>> +        ret = qemuDomainDetachNetDevice(driver, vm, dev, async);
>> +        break;
>> +    case VIR_DOMAIN_DEVICE_HOSTDEV:
>> +        ret = qemuDomainDetachHostDevice(driver, vm, dev, async);
>> +        break;
>> +    case VIR_DOMAIN_DEVICE_CHR:
>> +        ret = qemuDomainDetachChrDevice(driver, vm, dev->data.chr, async);
>> +        break;
>> +    case VIR_DOMAIN_DEVICE_RNG:
>> +        ret = qemuDomainDetachRNGDevice(driver, vm, dev->data.rng, async);
>> +        break;
>> +    case VIR_DOMAIN_DEVICE_MEMORY:
>> +        ret = qemuDomainDetachMemoryDevice(driver, vm, dev->data.memory, async);
>> +        break;
>> +    case VIR_DOMAIN_DEVICE_SHMEM:
>> +        ret = qemuDomainDetachShmemDevice(driver, vm, dev->data.shmem, async);
>> +        break;
>> +    case VIR_DOMAIN_DEVICE_WATCHDOG:
>> +        ret = qemuDomainDetachWatchdog(driver, vm, dev->data.watchdog, async);
>> +        break;
>> +    case VIR_DOMAIN_DEVICE_INPUT:
>> +        ret = qemuDomainDetachInputDevice(vm, dev->data.input, async);
>> +        break;
>> +    case VIR_DOMAIN_DEVICE_REDIRDEV:
>> +        ret = qemuDomainDetachRedirdevDevice(driver, vm, dev->data.redirdev, async);
>> +        break;
>> +
>> +    case VIR_DOMAIN_DEVICE_VSOCK:
>> +        ret = qemuDomainDetachVsockDevice(vm, dev->data.vsock, async);
>> +        break;
>> +
>> +    case VIR_DOMAIN_DEVICE_FS:
>> +    case VIR_DOMAIN_DEVICE_SOUND:
>> +    case VIR_DOMAIN_DEVICE_VIDEO:
>> +    case VIR_DOMAIN_DEVICE_GRAPHICS:
>> +    case VIR_DOMAIN_DEVICE_HUB:
>> +    case VIR_DOMAIN_DEVICE_SMARTCARD:
>> +    case VIR_DOMAIN_DEVICE_MEMBALLOON:
>> +    case VIR_DOMAIN_DEVICE_NVRAM:
>> +    case VIR_DOMAIN_DEVICE_NONE:
>> +    case VIR_DOMAIN_DEVICE_TPM:
>> +    case VIR_DOMAIN_DEVICE_PANIC:
>> +    case VIR_DOMAIN_DEVICE_IOMMU:
>> +    case VIR_DOMAIN_DEVICE_LAST:
>> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
>> +                       _("live detach of device '%s' is not supported"),
>> +                       virDomainDeviceTypeToString(dev->type));
>> +        break;
>> +    }
>> +
>> +    if (ret == 0)
>> +        ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE);
>> +
>> +    return ret;
>> +}
>> +
>> +
>>   static int
>>   qemuDomainRemoveVcpu(virQEMUDriverPtr driver,
>>                        virDomainObjPtr vm,
>> diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
>> index 7ac03b7810..61f732c506 100644
>> --- a/src/qemu/qemu_hotplug.h
>> +++ b/src/qemu/qemu_hotplug.h
>> @@ -79,10 +79,6 @@ int qemuDomainFindGraphicsIndex(virDomainDefPtr def,
>>   int qemuDomainAttachMemory(virQEMUDriverPtr driver,
>>                              virDomainObjPtr vm,
>>                              virDomainMemoryDefPtr mem);
>> -int qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
>> -                                 virDomainObjPtr vm,
>> -                                 virDomainMemoryDefPtr memdef,
>> -                                 bool async);
>>   int qemuDomainChangeGraphics(virQEMUDriverPtr driver,
>>                                virDomainObjPtr vm,
>>                                virDomainGraphicsDefPtr dev);
>> @@ -99,35 +95,6 @@ int qemuDomainChangeNetLinkState(virQEMUDriverPtr driver,
>>                                    virDomainObjPtr vm,
>>                                    virDomainNetDefPtr dev,
>>                                    int linkstate);
>> -int qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver,
>> -                                   virDomainObjPtr vm,
>> -                                   virDomainDeviceDefPtr dev,
>> -                                   bool async);
> [1]
>
>> -int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
>> -                                     virDomainObjPtr vm,
>> -                                     virDomainDeviceDefPtr dev,
>> -                                     bool async);
>> -int qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
>> -                              virDomainObjPtr vm,
>> -                              virDomainDeviceDefPtr dev,
>> -                              bool async);
>> -int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
>> -                               virDomainObjPtr vm,
>> -                               virDomainDeviceDefPtr dev,
>> -                               bool async);
>> -int qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
>> -                                virDomainObjPtr vm,
>> -                                virDomainShmemDefPtr dev,
>> -                                bool async);
>> -int qemuDomainDetachWatchdog(virQEMUDriverPtr driver,
>> -                             virDomainObjPtr vm,
>> -                             virDomainWatchdogDefPtr watchdog,
>> -                             bool async);
>> -
>> -int qemuDomainDetachRedirdevDevice(virQEMUDriverPtr driver,
>> -                                   virDomainObjPtr vm,
>> -                                   virDomainRedirdevDefPtr dev,
>> -                                   bool async);
>>   
>>   int qemuDomainAttachInputDevice(virQEMUDriverPtr driver,
>>                                   virDomainObjPtr vm,
>> @@ -140,23 +107,33 @@ int qemuDomainAttachVsockDevice(virQEMUDriverPtr driver,
>>   int qemuDomainAttachLease(virQEMUDriverPtr driver,
>>                             virDomainObjPtr vm,
>>                             virDomainLeaseDefPtr lease);
>> -int qemuDomainDetachLease(virQEMUDriverPtr driver,
>> -                          virDomainObjPtr vm,
>> -                          virDomainLeaseDefPtr lease);
>>   int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
>>                                 virDomainObjPtr vm,
>>                                 virDomainChrDefPtr chr);
>> -int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
>> -                              virDomainObjPtr vm,
>> -                              virDomainChrDefPtr chr,
>> -                              bool async);
>>   int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
>>                                 virDomainObjPtr vm,
>>                                 virDomainRNGDefPtr rng);
>> -int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
>> +
>> +int qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver,
>> +                                   virDomainObjPtr vm,
>> +                                   virDomainDeviceDefPtr dev,
>> +                                   bool async);
> These header movements don't belong to this patch.


Yeah, I suppose. Would you accept them in the same patch where I remove 
the prototypes for the functions that have become static? Or must that 
be separate as well?


>
>
>> +int qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
>> +                                virDomainObjPtr vm,
>> +                                virDomainShmemDefPtr dev,
>> +                                bool async);
>> +int qemuDomainDetachWatchdog(virQEMUDriverPtr driver,
>> +                             virDomainObjPtr vm,
>> +                             virDomainWatchdogDefPtr watchdog,
>> +                             bool async);
>> +int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
>>                                 virDomainObjPtr vm,
>> -                              virDomainRNGDefPtr rng,
>> +                              virDomainChrDefPtr chr,
>>                                 bool async);
>> +int qemuDomainDetachDeviceLive(virDomainObjPtr vm,
>> +                               virDomainDeviceDefPtr dev,
>> +                               virQEMUDriverPtr driver,
>> +                               bool async);
>
>>   
>>   void qemuDomainRemoveVcpuAlias(virQEMUDriverPtr driver,
>>                                  virDomainObjPtr vm,
>> @@ -191,11 +168,4 @@ int qemuDomainSetVcpuInternal(virQEMUDriverPtr driver,
>>                                 virBitmapPtr vcpus,
>>                                 bool state);
>>   
>> -int qemuDomainDetachInputDevice(virDomainObjPtr vm,
>> -                                virDomainInputDefPtr def,
>> -                                bool async);
>> -
>> -int qemuDomainDetachVsockDevice(virDomainObjPtr vm,
>> -                                virDomainVsockDefPtr dev,
>> -                                bool async);
>>   #endif /* LIBVIRT_QEMU_HOTPLUG_H */
>> -- 
>> 2.20.1
>>
>> --
>> libvir-list mailing list
>> libvir-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list





More information about the libvir-list mailing list