Re: [libvirt] [PATCHv10 1/6] libvirt/qemu - clean up for qemudDomainAt(de)tachDevice(Flags)

At 04/19/2011 03:40 PM, KAMEZAWA Hiroyuki Write: > > Centralize device modification in the more flexible APIs, to allow > future honoring of additional flags. Explicitly reject the > VIR_DOMAIN_DEVICE_MODIFY_FORCE flag on attach/detach. > > Based on Eric Blake&lt;eblake(a)redhat.com&gt;&#39;s work. > > From: Eric Blake <eblake(a)redhat.com&gt; > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; > > * src/qemu/qemu_driver.c (qemudDomainAttachDevice) > (qemudDomainAttachDeviceFlags): Swap bodies, and rename... Hmm, you do not rename the function(qemudDomainAttachDevice ===> qemuDomainAttachDevice)

> (qemuDomainAttachDevice, qemuDomainAttachDeviceFlags): to this. Hmm, it may be update not attach here.

> (qemudDomainDetachDevice, qemudDomainDetachDeviceFlags): Missing 'Likewise.'

> --- > src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++++--------------------- > 1 files changed, 29 insertions(+), 24 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index f6e503a..a8f3849 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -3786,8 +3786,8 @@ cleanup: > } > > > -static int qemudDomainAttachDevice(virDomainPtr dom, > - const char *xml) > +static int qemudDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, > + unsigned int flags) > { > struct qemud_driver *driver = dom->conn->privateData; > virDomainObjPtr vm; > @@ -3796,6 +3796,14 @@ static int qemudDomainAttachDevice(virDomainPtr dom, > virCgroupPtr cgroup = NULL; > int ret = -1; > > + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | > + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); > + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { > + qemuReportError(VIR_ERR_OPERATION_INVALID, > + "%s", _("cannot modify the persistent configuration of a domain")); > + return -1; > + } > + > qemuDriverLock(driver); > vm = virDomainFindByUUID(&driver->domains, dom->uuid); > if (!vm) { > @@ -3943,16 +3951,10 @@ cleanup: > return ret; > } > > -static int qemudDomainAttachDeviceFlags(virDomainPtr dom, > - const char *xml, > - unsigned int flags) { > - if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { > - qemuReportError(VIR_ERR_OPERATION_INVALID, > - "%s", _("cannot modify the persistent configuration of a domain")); > - return -1; > - } > - > - return qemudDomainAttachDevice(dom, xml); > +static int qemudDomainAttachDevice(virDomainPtr dom, const char *xml) > +{ > + return qemudDomainAttachDeviceFlags(dom, xml, > + VIR_DOMAIN_DEVICE_MODIFY_LIVE); > } > > > @@ -4078,14 +4080,23 @@ cleanup: > } > > > -static int qemudDomainDetachDevice(virDomainPtr dom, > - const char *xml) { > +static int qemudDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, > + unsigned int flags) > +{ > struct qemud_driver *driver = dom->conn->privateData; > virDomainObjPtr vm; > virBitmapPtr qemuCaps = NULL; > virDomainDeviceDefPtr dev = NULL; > int ret = -1; > > + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE| > + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); > + > + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { > + qemuReportError(VIR_ERR_OPERATION_INVALID, > + "%s", _("cannot modify the persistent configuration of a domain:")); > + return -1; > + } > qemuDriverLock(driver); > vm = virDomainFindByUUID(&driver->domains, dom->uuid); > if (!vm) { > @@ -4164,16 +4175,10 @@ cleanup: > return ret; > } > > -static int qemudDomainDetachDeviceFlags(virDomainPtr dom, > - const char *xml, > - unsigned int flags) { > - if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { > - qemuReportError(VIR_ERR_OPERATION_INVALID, > - "%s", _("cannot modify the persistent configuration of a domain")); > - return -1; > - } > - > - return qemudDomainDetachDevice(dom, xml); > +static int qemudDomainDetachDevice(virDomainPtr dom, const char *xml) > +{ > + return qemudDomainDetachDeviceFlags(dom, xml, > + VIR_DOMAIN_DEVICE_MODIFY_LIVE); > } > > static int qemudDomainGetAutostart(virDomainPtr dom,