[libvirt] [PATCHv6 1/3] libvirt/qemu - support persistent modification of devices.
Thank you for comments. This is a rebased/fixed one. =
From 5858b9d6b33a9d917576f6179be13a306272ff45 Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Date: Wed, 23 Mar 2011 17:45:12 +0900 Subject: [PATCHv6 1/3] libvirt/qemu - support persistent modification of devices.
Now, qemudDomainAttachDeviceFlags() and qemudDomainDetachDeviceFlags() doesn't support VIR_DOMAIN_DEVICE_MODIFY_CONFIG. By this, virsh's at(de)tatch-device --persistent cannot modify qemu config. (Xen allows it.) This patch is a base patch for adding support of devices in step by step manner. Following patches will add some device support. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Changelog v5->v6 - fixed Indentation - clean up. Changelog v4->v5: - fixed error codes at reporting Error. - fixed unlock code after calling qemuDomainObjBeginJobWithDriver() - fixed virDomainFindByUUID() to be virDomainFindByName() - fixed spelling, indent, etc.. - removed unnecessary checks. --- src/qemu/qemu_driver.c | 137 +++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 124 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6f296c9..468361b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4140,16 +4140,124 @@ 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")); +/* + * Attach a device given by XML, the change will be persistent + * and domain XML definition file is updated. + */ +static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, + virDomainDeviceDefPtr newdev) +{ + + switch(newdev->type) { + default: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Sorry, the device is not supported for now")); + return -1; + } + + return 0; +} + +static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, + virDomainDeviceDefPtr device) +{ + switch(device->type) { + default: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Sorry, the device is not supported for now")); return -1; } + return 0; +} + +static int qemuDomainModifyDevicePersistent(virDomainPtr dom, + const char *xml, + unsigned int attach, unsigned int flags) +{ + struct qemud_driver *driver; + virDomainDeviceDefPtr device; + virDomainDefPtr vmdef; + virDomainObjPtr vm; + int ret = -1; + + /* + * When both of MODIFY_CONFIG and MODIFY_LIVE is specified at the same time, + * return error for now. We should support this later. + */ + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Now, cannot modify alive domain and its definition " + "at the same time.")); + return -1; + } + + driver = dom->conn->privateData; + qemuDriverLock(driver); + vm = virDomainFindByName(&driver->domains, dom->name); + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with name : '%s'"), + dom->name); + goto unlock_out; + } + + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + goto unlock_out; + + if (virDomainObjIsActive(vm)) { + /* + * For now, just allow updating inactive domains. Further development + * will allow updating both active domain and its config file at + * the same time. + */ + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("unsupported to update active domain's definition.")); + goto endjob; + } + + vmdef = virDomainObjGetPersistentDef(driver->caps, vm); - return qemudDomainAttachDevice(dom, xml); + if (!vmdef) + goto endjob; + + device = virDomainDeviceDefParse(driver->caps, + vmdef, xml, VIR_DOMAIN_XML_INACTIVE); + if (!device) + goto endjob; + + if (attach) + ret = qemuDomainAttachDevicePersistent(vmdef, device); + else + ret = qemuDomainDetachDevicePersistent(vmdef, device); + + if (!ret) /* save the result */ + ret = virDomainSaveConfig(driver->configDir, vmdef); + + virDomainDeviceDefFree(device); + +endjob: + if (qemuDomainObjEndJob(vm) == 0) + vm = NULL; +unlock_out: + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + +static int qemudDomainAttachDeviceFlags(virDomainPtr dom, + const char *xml, + unsigned int flags) +{ + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) + return qemuDomainModifyDevicePersistent(dom, xml, 1, flags); + + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) + return qemudDomainAttachDevice(dom, xml); + + qemuReportError(VIR_ERR_INVALID_ARG, + _("bad flag: %x not supported"), flags); + + return -1; } @@ -4364,13 +4472,16 @@ cleanup: 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); + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) + return qemuDomainModifyDevicePersistent(dom, xml, 0, flags); + + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) + return qemudDomainDetachDevice(dom, xml); + + qemuReportError(VIR_ERR_INVALID_ARG, + _("bad flag: %x not supported now"), flags); + return -1; } static int qemudDomainGetAutostart(virDomainPtr dom, -- 1.7.4.1
From 1d2630896a950b87043c9e77fdbcdc23626098ee Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Date: Thu, 24 Mar 2011 10:05:18 +0900 Subject: [PATCHv6 2/3] libvirt/qemu - support persistent modification of qemu disks
support changes of disks by VIR_DOMAIN_DEVICE_MODIFY_CONFIG for qemu. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Changelog v5->v6: - fixed pci address/device controller assign failure case. Changelog v4->v5: - moved some functions to domain_conf.c - added virDomainDefAddImplicitControllers() for ide/scsi - report OOM. - clean up. --- src/conf/domain_conf.c | 22 +++++++++++++++++++ src/conf/domain_conf.h | 3 +- src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 51 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 76 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c637300..1bf8fbe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4859,6 +4859,19 @@ virVirtualPortProfileFormat(virBufferPtr buf, virBufferVSprintf(buf, "%s</virtualport>\n", indent); } +int virDomainDiskIndexByName(virDomainDefPtr def, const char *name) +{ + virDomainDiskDefPtr vdisk; + int i; + + for (i = 0; i < def->ndisks; i++) { + vdisk = def->disks[i]; + if (STREQ(vdisk->dst, name)) + return i; + } + return -1; +} + int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk) { @@ -4930,6 +4943,15 @@ void virDomainDiskRemove(virDomainDefPtr def, size_t i) } } +int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name) +{ + int i = virDomainDiskIndexByName(def, name); + if (i < 0) + return -1; + virDomainDiskRemove(def, i); + return 0; +} + int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cbd0f98..236ad04 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1258,7 +1258,7 @@ int virDomainCpuSetParse(const char **str, int maxcpu); char *virDomainCpuSetFormat(char *cpuset, int maxcpu); - +int virDomainDiskIndexByName(virDomainDefPtr def, const char *name); int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk); void virDomainDiskInsertPreAlloced(virDomainDefPtr def, @@ -1266,6 +1266,7 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def, int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def); void virDomainDiskRemove(virDomainDefPtr def, size_t i); +int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name); int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 55be36e..9ced196 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -243,11 +243,13 @@ virDomainDiskDefFree; virDomainDiskDeviceTypeToString; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; +virDomainDiskIndexByName; virDomainDiskInsert; virDomainDiskInsertPreAlloced; virDomainDiskIoTypeFromString; virDomainDiskIoTypeToString; virDomainDiskRemove; +virDomainDiskRemoveByName; virDomainDiskTypeFromString; virDomainDiskTypeToString; virDomainFSDefFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 468361b..386b654 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4140,6 +4140,24 @@ cleanup: return ret; } +static int qemuDomainDeviceAddressFixup(virDomainDefPtr vmdef, bool pci) +{ + if (pci) { + if (qemuDomainAssignPCIAddresses(vmdef)) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("device or domain PCI addresses.")); + return -1; + } + } else { + if (virDomainDefAddImplicitControllers(vmdef) < 0) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("device address or domain device controllers.")); + return -1; + } + } + return 0; +} + /* * Attach a device given by XML, the change will be persistent * and domain XML definition file is updated. @@ -4147,21 +4165,52 @@ cleanup: static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr newdev) { + virDomainDiskDefPtr disk; + bool pci; + int ret; switch(newdev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = newdev->data.disk; + if (virDomainDiskIndexByName(vmdef, disk->dst) >= 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s already exists."), disk->dst); + return -1; + } + if (virDomainDiskInsert(vmdef, disk)) { + virReportOOMError(); + return -1; + } + pci = (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO); + ret = qemuDomainDeviceAddressFixup(vmdef, pci); + if (ret < 0) + virDomainDiskRemoveByName(vmdef, disk->dst); + else + newdev->data.disk = NULL; + break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Sorry, the device is not supported for now")); return -1; } - return 0; + return ret; } static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr device) { + virDomainDiskDefPtr disk; + switch(device->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = device->data.disk; + if (virDomainDiskRemoveByName(vmdef, disk->dst)) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("no target device %s"), disk->dst); + return -1; + } + break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Sorry, the device is not supported for now")); -- 1.7.4.1
At 03/24/2011 09:31 AM, KAMEZAWA Hiroyuki Write:
From 1d2630896a950b87043c9e77fdbcdc23626098ee Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Date: Thu, 24 Mar 2011 10:05:18 +0900 Subject: [PATCHv6 2/3] libvirt/qemu - support persistent modification of qemu disks
support changes of disks by VIR_DOMAIN_DEVICE_MODIFY_CONFIG for qemu.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Changelog v5->v6: - fixed pci address/device controller assign failure case.
Changelog v4->v5: - moved some functions to domain_conf.c - added virDomainDefAddImplicitControllers() for ide/scsi - report OOM. - clean up. --- src/conf/domain_conf.c | 22 +++++++++++++++++++ src/conf/domain_conf.h | 3 +- src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 51 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 76 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c637300..1bf8fbe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4859,6 +4859,19 @@ virVirtualPortProfileFormat(virBufferPtr buf, virBufferVSprintf(buf, "%s</virtualport>\n", indent); }
+int virDomainDiskIndexByName(virDomainDefPtr def, const char *name) +{ + virDomainDiskDefPtr vdisk; + int i; + + for (i = 0; i < def->ndisks; i++) { + vdisk = def->disks[i]; + if (STREQ(vdisk->dst, name)) + return i; + } + return -1; +} + int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk) { @@ -4930,6 +4943,15 @@ void virDomainDiskRemove(virDomainDefPtr def, size_t i) } }
+int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name) +{ + int i = virDomainDiskIndexByName(def, name); + if (i < 0) + return -1; + virDomainDiskRemove(def, i); + return 0; +} +
int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cbd0f98..236ad04 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1258,7 +1258,7 @@ int virDomainCpuSetParse(const char **str, int maxcpu); char *virDomainCpuSetFormat(char *cpuset, int maxcpu); - +int virDomainDiskIndexByName(virDomainDefPtr def, const char *name); int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk); void virDomainDiskInsertPreAlloced(virDomainDefPtr def, @@ -1266,6 +1266,7 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def, int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def);
void virDomainDiskRemove(virDomainDefPtr def, size_t i); +int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name);
int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 55be36e..9ced196 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -243,11 +243,13 @@ virDomainDiskDefFree; virDomainDiskDeviceTypeToString; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; +virDomainDiskIndexByName; virDomainDiskInsert; virDomainDiskInsertPreAlloced; virDomainDiskIoTypeFromString; virDomainDiskIoTypeToString; virDomainDiskRemove; +virDomainDiskRemoveByName; virDomainDiskTypeFromString; virDomainDiskTypeToString; virDomainFSDefFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 468361b..386b654 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4140,6 +4140,24 @@ cleanup: return ret; }
+static int qemuDomainDeviceAddressFixup(virDomainDefPtr vmdef, bool pci) +{ + if (pci) { + if (qemuDomainAssignPCIAddresses(vmdef)) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("device or domain PCI addresses.")); + return -1; + } + } else { + if (virDomainDefAddImplicitControllers(vmdef) < 0) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("device address or domain device controllers.")); + return -1; + } + }
Do not report error when qemuDomainAssignPCIAddresses() or virDomainDefAddImplicitControllers() failed, because we have reported the error in these functions.
+ return 0; +} + /* * Attach a device given by XML, the change will be persistent * and domain XML definition file is updated. @@ -4147,21 +4165,52 @@ cleanup: static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr newdev) { + virDomainDiskDefPtr disk; + bool pci; + int ret;
switch(newdev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = newdev->data.disk; + if (virDomainDiskIndexByName(vmdef, disk->dst) >= 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s already exists."), disk->dst); + return -1; + } + if (virDomainDiskInsert(vmdef, disk)) { + virReportOOMError(); + return -1; + } + pci = (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO); + ret = qemuDomainDeviceAddressFixup(vmdef, pci); + if (ret < 0) + virDomainDiskRemoveByName(vmdef, disk->dst); + else + newdev->data.disk = NULL; + break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Sorry, the device is not supported for now")); return -1; }
- return 0; + return ret; }
static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr device) { + virDomainDiskDefPtr disk; + switch(device->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = device->data.disk; + if (virDomainDiskRemoveByName(vmdef, disk->dst)) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("no target device %s"), disk->dst); + return -1; + } + break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Sorry, the device is not supported for now"));
On Thu, 24 Mar 2011 15:44:05 +0800 Wen Congyang <wency@cn.fujitsu.com> wrote:
At 03/24/2011 09:31 AM, KAMEZAWA Hiroyuki Write:
From 1d2630896a950b87043c9e77fdbcdc23626098ee Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Date: Thu, 24 Mar 2011 10:05:18 +0900 Subject: [PATCHv6 2/3] libvirt/qemu - support persistent modification of qemu disks
support changes of disks by VIR_DOMAIN_DEVICE_MODIFY_CONFIG for qemu.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Changelog v5->v6: - fixed pci address/device controller assign failure case.
Changelog v4->v5: - moved some functions to domain_conf.c - added virDomainDefAddImplicitControllers() for ide/scsi - report OOM. - clean up. --- src/conf/domain_conf.c | 22 +++++++++++++++++++ src/conf/domain_conf.h | 3 +- src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 51 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 76 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c637300..1bf8fbe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4859,6 +4859,19 @@ virVirtualPortProfileFormat(virBufferPtr buf, virBufferVSprintf(buf, "%s</virtualport>\n", indent); }
+int virDomainDiskIndexByName(virDomainDefPtr def, const char *name) +{ + virDomainDiskDefPtr vdisk; + int i; + + for (i = 0; i < def->ndisks; i++) { + vdisk = def->disks[i]; + if (STREQ(vdisk->dst, name)) + return i; + } + return -1; +} + int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk) { @@ -4930,6 +4943,15 @@ void virDomainDiskRemove(virDomainDefPtr def, size_t i) } }
+int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name) +{ + int i = virDomainDiskIndexByName(def, name); + if (i < 0) + return -1; + virDomainDiskRemove(def, i); + return 0; +} +
int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cbd0f98..236ad04 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1258,7 +1258,7 @@ int virDomainCpuSetParse(const char **str, int maxcpu); char *virDomainCpuSetFormat(char *cpuset, int maxcpu); - +int virDomainDiskIndexByName(virDomainDefPtr def, const char *name); int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk); void virDomainDiskInsertPreAlloced(virDomainDefPtr def, @@ -1266,6 +1266,7 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def, int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def);
void virDomainDiskRemove(virDomainDefPtr def, size_t i); +int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name);
int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 55be36e..9ced196 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -243,11 +243,13 @@ virDomainDiskDefFree; virDomainDiskDeviceTypeToString; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; +virDomainDiskIndexByName; virDomainDiskInsert; virDomainDiskInsertPreAlloced; virDomainDiskIoTypeFromString; virDomainDiskIoTypeToString; virDomainDiskRemove; +virDomainDiskRemoveByName; virDomainDiskTypeFromString; virDomainDiskTypeToString; virDomainFSDefFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 468361b..386b654 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4140,6 +4140,24 @@ cleanup: return ret; }
+static int qemuDomainDeviceAddressFixup(virDomainDefPtr vmdef, bool pci) +{ + if (pci) { + if (qemuDomainAssignPCIAddresses(vmdef)) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("device or domain PCI addresses.")); + return -1; + } + } else { + if (virDomainDefAddImplicitControllers(vmdef) < 0) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("device address or domain device controllers.")); + return -1; + } + }
Do not report error when qemuDomainAssignPCIAddresses() or virDomainDefAddImplicitControllers() failed, because we have reported the error in these functions.
Hmm, I didn't see any error message at failure caused by this...I'll check again. Thanks, -Kame
At 03/24/2011 04:35 PM, KAMEZAWA Hiroyuki Write:
On Thu, 24 Mar 2011 15:44:05 +0800 Wen Congyang <wency@cn.fujitsu.com> wrote:
At 03/24/2011 09:31 AM, KAMEZAWA Hiroyuki Write:
From 1d2630896a950b87043c9e77fdbcdc23626098ee Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Date: Thu, 24 Mar 2011 10:05:18 +0900 Subject: [PATCHv6 2/3] libvirt/qemu - support persistent modification of qemu disks
support changes of disks by VIR_DOMAIN_DEVICE_MODIFY_CONFIG for qemu.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Changelog v5->v6: - fixed pci address/device controller assign failure case.
Changelog v4->v5: - moved some functions to domain_conf.c - added virDomainDefAddImplicitControllers() for ide/scsi - report OOM. - clean up. --- src/conf/domain_conf.c | 22 +++++++++++++++++++ src/conf/domain_conf.h | 3 +- src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 51 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 76 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c637300..1bf8fbe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4859,6 +4859,19 @@ virVirtualPortProfileFormat(virBufferPtr buf, virBufferVSprintf(buf, "%s</virtualport>\n", indent); }
+int virDomainDiskIndexByName(virDomainDefPtr def, const char *name) +{ + virDomainDiskDefPtr vdisk; + int i; + + for (i = 0; i < def->ndisks; i++) { + vdisk = def->disks[i]; + if (STREQ(vdisk->dst, name)) + return i; + } + return -1; +} + int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk) { @@ -4930,6 +4943,15 @@ void virDomainDiskRemove(virDomainDefPtr def, size_t i) } }
+int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name) +{ + int i = virDomainDiskIndexByName(def, name); + if (i < 0) + return -1; + virDomainDiskRemove(def, i); + return 0; +} +
int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cbd0f98..236ad04 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1258,7 +1258,7 @@ int virDomainCpuSetParse(const char **str, int maxcpu); char *virDomainCpuSetFormat(char *cpuset, int maxcpu); - +int virDomainDiskIndexByName(virDomainDefPtr def, const char *name); int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk); void virDomainDiskInsertPreAlloced(virDomainDefPtr def, @@ -1266,6 +1266,7 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def, int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def);
void virDomainDiskRemove(virDomainDefPtr def, size_t i); +int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name);
int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 55be36e..9ced196 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -243,11 +243,13 @@ virDomainDiskDefFree; virDomainDiskDeviceTypeToString; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; +virDomainDiskIndexByName; virDomainDiskInsert; virDomainDiskInsertPreAlloced; virDomainDiskIoTypeFromString; virDomainDiskIoTypeToString; virDomainDiskRemove; +virDomainDiskRemoveByName; virDomainDiskTypeFromString; virDomainDiskTypeToString; virDomainFSDefFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 468361b..386b654 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4140,6 +4140,24 @@ cleanup: return ret; }
+static int qemuDomainDeviceAddressFixup(virDomainDefPtr vmdef, bool pci) +{ + if (pci) { + if (qemuDomainAssignPCIAddresses(vmdef)) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("device or domain PCI addresses.")); + return -1; + } + } else { + if (virDomainDefAddImplicitControllers(vmdef) < 0) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("device address or domain device controllers.")); + return -1; + } + }
Do not report error when qemuDomainAssignPCIAddresses() or virDomainDefAddImplicitControllers() failed, because we have reported the error in these functions.
Hmm, I didn't see any error message at failure caused by this...I'll check again.
Sometimes, qemuDomainAssignPCIAddress() failed, and it does not report error. qemuDomainAssignPCIAddress() calls qemuDomainPCIAddressSetCreate() that calls virDomainDeviceInfoIterate() that calls qemuCollectPCIAddress() that calls virHashAddEntry() that calls virHashAddOrUpdateEntry() When the pci address of two drivers are the same, virHashAddOrUpdateEntry() will fail. if (found) { if (is_update) { if (table->dataFree) table->dataFree(insert->payload, insert->name); insert->payload = userdata; return (0); } else { return (-1); <==== we do not report error here. } }
Thanks, -Kame
On Thu, 24 Mar 2011 16:49:29 +0800 Wen Congyang <wency@cn.fujitsu.com> wrote:
Sometimes, qemuDomainAssignPCIAddress() failed, and it does not report error.
qemuDomainAssignPCIAddress() calls qemuDomainPCIAddressSetCreate() that calls virDomainDeviceInfoIterate() that calls qemuCollectPCIAddress() that calls virHashAddEntry() that calls virHashAddOrUpdateEntry()
When the pci address of two drivers are the same, virHashAddOrUpdateEntry() will fail. if (found) { if (is_update) { if (table->dataFree) table->dataFree(insert->payload, insert->name); insert->payload = userdata; return (0); } else { return (-1); <==== we do not report error here. } }
Then, do you have recomendation ? Just retuns error without any hint is bad behavior. Thanks, -Kame
On Thu, 24 Mar 2011 17:35:19 +0900 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
On Thu, 24 Mar 2011 15:44:05 +0800 Wen Congyang <wency@cn.fujitsu.com> wrote:
Do not report error when qemuDomainAssignPCIAddresses() or virDomainDefAddImplicitControllers() failed, because we have reported the error in these functions.
Hmm, I didn't see any error message at failure caused by this...I'll check again.
It seems qemuDomainAssignPCIAddresses() doesn't return error at address confliction, it just report it by VIR_DEBUG("PCI addr %s already in use", addr), IIUC. Thanks, -Kame
From fa4ecbfabe3ec0bcf120bedf20583a2045430f65 Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Date: Thu, 24 Mar 2011 10:26:56 +0900 Subject: [PATCHv6 3/3] libvirt/qemu - support persistent modification of qemu nics. support changes of interfaces by VIR_DOMAIN_DEVICE_MODIFY_CONFIG for qemu.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Changelog v5->v6 - fixed pci address assign failure case. Changelog v4->v5 - fixed mac handling. - moved some functions to domain_conf.c - clean up. --- src/conf/domain_conf.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 5 ++++ src/libvirt_private.syms | 4 +++ src/qemu/qemu_driver.c | 38 +++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1bf8fbe..3c5c87c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4952,6 +4952,62 @@ int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name) return 0; } +int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net) +{ + if (VIR_REALLOC_N(def->nets, def->nnets) < 0) + return -1; + def->nets[def->nnets] = net; + def->nnets++; + return 0; +} + +int virDomainNetIndexByMac(virDomainDefPtr def, const unsigned char*mac) +{ + int i; + + for (i = 0; i < def->nnets; i++) + if (memcmp(def->nets[i]->mac, mac, VIR_MAC_BUFLEN)) + return i; + return -1; +} + +int virDomainNetIndexByIfname(virDomainDefPtr def, const char*name) +{ + int i; + + for (i = 0; i < def->nnets; i++) { + if (def->nets[i]->ifname && STREQ(def->nets[i]->ifname, name)) + return i; + } + return -1; +} + +static void virDomainNetRemove(virDomainDefPtr def, size_t i) +{ + if (def->nnets > 1) { + memmove(def->nets + i, + def->nets + i + 1, + sizeof(*def->nets) * (def->nnets - (i + 1))); + def->nnets--; + if (VIR_REALLOC_N(def->nets, def->nnets) < 0) { + /* ignore harmless */ + } + } else { + VIR_FREE(def->nets); + def->nnets = 0; + } +} + +int virDomainNetRemoveByMac(virDomainDefPtr def, const unsigned char *mac) +{ + int i; + + i = virDomainNetIndexByMac(def, mac); + if (i < 0) + return -1; + virDomainNetRemove(def, i); + return 0; +} int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 236ad04..19a521c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1268,6 +1268,11 @@ int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def); void virDomainDiskRemove(virDomainDefPtr def, size_t i); int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name); +int virDomainNetIndexByIfname(virDomainDefPtr def, const char *ifname); +int virDomainNetIndexByMac(virDomainDefPtr def, const unsigned char *mac); +int virDomainNetInsert(virDomainDefPtr dev, virDomainNetDefPtr net); +int virDomainNetRemoveByMac(virDomainDefPtr def, const unsigned char *mac); + int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); void virDomainControllerInsertPreAlloced(virDomainDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9ced196..50cdebf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -277,6 +277,10 @@ virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; virDomainNetDefFree; virDomainNetTypeToString; +virDomainNetIndexByIfname; +virDomainNetIndexByMac; +virDomainNetInsert; +virDomainNetRemoveByMac; virDomainObjAssignDef; virDomainObjSetDefTransient; virDomainObjGetPersistentDef; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 386b654..371dee6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4166,6 +4166,7 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr newdev) { virDomainDiskDefPtr disk; + virDomainNetDefPtr net; bool pci; int ret; @@ -4188,6 +4189,31 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, else newdev->data.disk = NULL; break; + case VIR_DOMAIN_DEVICE_NET: + net = newdev->data.net; + /* check mac */ + if (virDomainNetIndexByMac(vmdef, net->mac) >= 0) { + char macbuf[VIR_MAC_STRING_BUFLEN]; + + virFormatMacAddr(net->mac, macbuf); + qemuReportError(VIR_ERR_INVALID_ARG, + _("target mac %s already exists."), macbuf); + } + /* Sanity check : test ifname confliction. */ + if (net->ifname && virDomainNetIndexByIfname(vmdef, net->ifname) >= 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target NIC %s already exists."), net->ifname); + } + if (virDomainNetInsert(vmdef, net)) { + virReportOOMError(); + return -1; + } + ret = qemuDomainDeviceAddressFixup(vmdef, true); + if (ret < 0) + virDomainNetRemoveByMac(vmdef, net->mac); + else + newdev->data.net = NULL; + break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Sorry, the device is not supported for now")); @@ -4201,6 +4227,7 @@ static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr device) { virDomainDiskDefPtr disk; + virDomainNetDefPtr net; switch(device->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -4211,6 +4238,17 @@ static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, return -1; } break; + case VIR_DOMAIN_DEVICE_NET: + net = device->data.net; + if (virDomainNetRemoveByMac(vmdef, net->mac)) { + char macbuf[VIR_MAC_STRING_BUFLEN]; + + virFormatMacAddr(net->mac, macbuf); + qemuReportError(VIR_ERR_INVALID_ARG, + _("no interface with mac %s"), macbuf); + return -1; + } + break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Sorry, the device is not supported for now")); -- 1.7.4.1
At 03/24/2011 09:33 AM, KAMEZAWA Hiroyuki Write:
From fa4ecbfabe3ec0bcf120bedf20583a2045430f65 Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Date: Thu, 24 Mar 2011 10:26:56 +0900 Subject: [PATCHv6 3/3] libvirt/qemu - support persistent modification of qemu nics. support changes of interfaces by VIR_DOMAIN_DEVICE_MODIFY_CONFIG for qemu.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Changelog v5->v6 - fixed pci address assign failure case. Changelog v4->v5 - fixed mac handling. - moved some functions to domain_conf.c - clean up. --- src/conf/domain_conf.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 5 ++++ src/libvirt_private.syms | 4 +++ src/qemu/qemu_driver.c | 38 +++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1bf8fbe..3c5c87c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4952,6 +4952,62 @@ int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name) return 0; }
+int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net) +{ + if (VIR_REALLOC_N(def->nets, def->nnets) < 0) + return -1;
OOPS, s/def->nnets/def->nnets+1/
+ def->nets[def->nnets] = net; + def->nnets++; + return 0; +} + +int virDomainNetIndexByMac(virDomainDefPtr def, const unsigned char*mac) +{ + int i; + + for (i = 0; i < def->nnets; i++) + if (memcmp(def->nets[i]->mac, mac, VIR_MAC_BUFLEN)) + return i; + return -1; +} + +int virDomainNetIndexByIfname(virDomainDefPtr def, const char*name) +{ + int i; + + for (i = 0; i < def->nnets; i++) { + if (def->nets[i]->ifname && STREQ(def->nets[i]->ifname, name)) + return i; + } + return -1; +} + +static void virDomainNetRemove(virDomainDefPtr def, size_t i) +{ + if (def->nnets > 1) { + memmove(def->nets + i, + def->nets + i + 1, + sizeof(*def->nets) * (def->nnets - (i + 1))); + def->nnets--; + if (VIR_REALLOC_N(def->nets, def->nnets) < 0) { + /* ignore harmless */ + } + } else { + VIR_FREE(def->nets); + def->nnets = 0; + } +} + +int virDomainNetRemoveByMac(virDomainDefPtr def, const unsigned char *mac) +{ + int i; + + i = virDomainNetIndexByMac(def, mac); + if (i < 0) + return -1; + virDomainNetRemove(def, i); + return 0; +}
int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 236ad04..19a521c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1268,6 +1268,11 @@ int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def); void virDomainDiskRemove(virDomainDefPtr def, size_t i); int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name);
+int virDomainNetIndexByIfname(virDomainDefPtr def, const char *ifname); +int virDomainNetIndexByMac(virDomainDefPtr def, const unsigned char *mac); +int virDomainNetInsert(virDomainDefPtr dev, virDomainNetDefPtr net); +int virDomainNetRemoveByMac(virDomainDefPtr def, const unsigned char *mac); + int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); void virDomainControllerInsertPreAlloced(virDomainDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9ced196..50cdebf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -277,6 +277,10 @@ virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; virDomainNetDefFree; virDomainNetTypeToString; +virDomainNetIndexByIfname; +virDomainNetIndexByMac; +virDomainNetInsert; +virDomainNetRemoveByMac; virDomainObjAssignDef; virDomainObjSetDefTransient; virDomainObjGetPersistentDef; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 386b654..371dee6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4166,6 +4166,7 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr newdev) { virDomainDiskDefPtr disk; + virDomainNetDefPtr net; bool pci; int ret;
@@ -4188,6 +4189,31 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, else newdev->data.disk = NULL; break; + case VIR_DOMAIN_DEVICE_NET: + net = newdev->data.net; + /* check mac */ + if (virDomainNetIndexByMac(vmdef, net->mac) >= 0) { + char macbuf[VIR_MAC_STRING_BUFLEN]; + + virFormatMacAddr(net->mac, macbuf); + qemuReportError(VIR_ERR_INVALID_ARG, + _("target mac %s already exists."), macbuf);
'return -1' is misssed.
+ } + /* Sanity check : test ifname confliction. */ + if (net->ifname && virDomainNetIndexByIfname(vmdef, net->ifname) >= 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target NIC %s already exists."), net->ifname);
the same as above.
+ } + if (virDomainNetInsert(vmdef, net)) { + virReportOOMError(); + return -1; + } + ret = qemuDomainDeviceAddressFixup(vmdef, true); + if (ret < 0) + virDomainNetRemoveByMac(vmdef, net->mac); + else + newdev->data.net = NULL; + break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Sorry, the device is not supported for now")); @@ -4201,6 +4227,7 @@ static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr device) { virDomainDiskDefPtr disk; + virDomainNetDefPtr net;
switch(device->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -4211,6 +4238,17 @@ static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, return -1; } break; + case VIR_DOMAIN_DEVICE_NET: + net = device->data.net; + if (virDomainNetRemoveByMac(vmdef, net->mac)) { + char macbuf[VIR_MAC_STRING_BUFLEN]; + + virFormatMacAddr(net->mac, macbuf); + qemuReportError(VIR_ERR_INVALID_ARG, + _("no interface with mac %s"), macbuf); + return -1; + } + break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Sorry, the device is not supported for now"));
participants (2)
-
KAMEZAWA Hiroyuki -
Wen Congyang