8:36 a.m.
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(a)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(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list