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

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&gt; > > --- > > 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.

>

> > { > > + 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?