[libvirt] [PATCH 1/2] libvirt/qemu - support updating inactive domains (take 2)
From d6c65102224c493f67008b6521b0115d798f5a67 Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa@bluextal.(none)> Date: Thu, 24 Feb 2011 12:14:25 +0900 Subject: [PATCH 1/2] libvirt/qemu - support updating inactive domains.
Now, virsh attach-disk/detach-disk has --persistent option and it can update XML definition of inactive domains. But, it's only supported in Xen. This patch adds support for attach-disk/detach-disk for qemu. Changelog v1->v2: - fixed TABs - fixed PCI address assignment for VIRTIO driver. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> --- src/qemu/qemu_driver.c | 168 +++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 158 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8b15a3e..e75478a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4085,14 +4085,164 @@ cleanup: return ret; } -static int qemudDomainAttachDeviceFlags(virDomainPtr dom, +static int qemuDomainFindDiskByName(virDomainDefPtr vmdef, const char *name) +{ + virDomainDiskDefPtr vdisk; + int i; + + for (i = 0; i < vmdef->ndisks; i++) { + vdisk = vmdef->disks[i]; + if (!strcmp(vdisk->dst, name)) + return i; + } + return -1; +} +/* + * 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) +{ + virDomainDiskDefPtr disk; + + /* At first, check device confliction */ + switch(newdev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = newdev->data.disk; + if (qemuDomainFindDiskByName(vmdef, disk->dst) >= 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s already exists."), disk->dst); + return -1; + } + + if (virDomainDiskInsert(vmdef, disk)) { + virReportOOMError(); + return -1; + } + if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { + /* + * Address/Drive information is NULL. If virtio, PCI address + * shoule be fixed. Other devices as IDE, SCSI...will get ID + * automatically + */ + qemuDomainAssignPCIAddresses(vmdef); + + } + newdev->data.disk = NULL; + break; + default: + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("Sorry, the device is not suppored for now")); + return -1; + } + + return 0; +} + +static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, + virDomainDeviceDefPtr device) +{ + int x; + virDomainDiskDefPtr disk; + + switch(device->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = device->data.disk; + x = qemuDomainFindDiskByName(vmdef, disk->dst); + if (x < 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s doesn't exist."), disk->dst); + return -1; + } + virDomainDiskRemove(vmdef, x); + break; + default: + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("Sorry, the device is not suppored for now")); + return -1; + } + return 0; +} + +static int qemuDomainModifyDevicePersistent(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")); + unsigned int attach) +{ + struct qemud_driver *driver; + virDomainDeviceDefPtr device; + virDomainDefPtr vmdef; + virDomainObjPtr vm; + int ret = -1; + + if (!dom || !dom->conn || !dom->name || !xml) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("internal error : %s"), __FUNCTION__); + return -1; + } + + if (dom->conn->flags & VIR_CONNECT_RO) return -1; + + driver = dom->conn->privateData; + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, _("cannot find domain '%s'"), + dom->name); + goto unlock_out; + } + + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) { + /* + * 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_INVALID_ARG, + _("cannot update alive domain : %s"), __FUNCTION__); + goto endjob; + } + vmdef = virDomainObjGetPersistentDef(driver->caps, vm); + + if (!vmdef) + goto endjob; + + device = virDomainDeviceDefParse(driver->caps, + vmdef, xml, VIR_DOMAIN_XML_INACTIVE); + if (!device) + goto endjob; + + if (attach) { + ret = qemuDomainAttachDevicePersistent(vmdef, device); + if (ret < 0) + goto out; + } else { + ret = qemuDomainDetachDevicePersistent(vmdef, device); + if (ret < 0) + goto out; } + ret = virDomainSaveConfig(driver->configDir, vmdef); + +out: + virDomainDeviceDefFree(device); +endjob: + if (qemuDomainObjEndJob(vm) == 0) + vm = NULL; + if (vm) + virDomainObjUnlock(vm); + /* Note: insert of newdev is done by copy */ +unlock_out: + 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); return qemudDomainAttachDevice(dom, xml); } @@ -4306,11 +4456,9 @@ 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; - } + + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) + return qemuDomainModifyDevicePersistent(dom, xml, 0); return qemudDomainDetachDevice(dom, xml); } -- 1.7.1
From b06da6d9b4ee996046af72a81c89b90852372e53 Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa@bluextal.(none)> Date: Thu, 24 Feb 2011 13:08:54 +0900 Subject: [PATCH 2/2] libvirt/qemu : support attach/detach-interface --persistent with qemu
Now, virsh attach/detach-interface have --persistent option for updating inactive domain but it's only supported in Xen. This patch adds support for qemu. Changelog v1->v2: - fixed TABs - fixed header file, type of a function Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> --- src/conf/domain_conf.c | 50 ++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 ++ src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b97c1f0..660573b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4924,6 +4924,56 @@ void virDomainDiskRemove(virDomainDefPtr def, size_t i) } } +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; +} + +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 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; +} + +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 virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 30aeccc..396dfbc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1270,6 +1270,9 @@ int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def); void virDomainDiskRemove(virDomainDefPtr def, size_t i); +void virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net); +void virDomainNetRemove(virDomainDefPtr def, size_t i); + int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); void virDomainControllerInsertPreAlloced(virDomainDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 797a670..eef5266 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -245,6 +245,8 @@ virDomainDiskIoTypeToString; virDomainDiskRemove; virDomainDiskTypeFromString; virDomainDiskTypeToString; +virDomainNetInsert; +virDomainNetRemove; virDomainFSDefFree; virDomainFindByID; virDomainFindByName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e75478a..00a240b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4097,6 +4097,22 @@ static int qemuDomainFindDiskByName(virDomainDefPtr vmdef, const char *name) } return -1; } + +static int qemuDomainFindNetByName(virDomainDefPtr vmdef, + const unsigned char *mac, const char *ifname) +{ + virDomainNetDefPtr net; + int i; + + for (i = 0; i < vmdef->nnets; i++) { + net = vmdef->nets[i]; + if (!strcmp((char*)net->mac, (char*)mac)) + return i; + if (ifname && !strcmp((char*)net->ifname, (char*)ifname)) + return i; + } + return -1; +} /* * Attach a device given by XML, the change will be persistent * and domain XML definition file is updated. @@ -4105,6 +4121,7 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr newdev) { virDomainDiskDefPtr disk; + virDomainNetDefPtr net; /* At first, check device confliction */ switch(newdev->type) { @@ -4131,6 +4148,22 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, } newdev->data.disk = NULL; break; + case VIR_DOMAIN_DEVICE_NET: + net = newdev->data.net; + if (qemuDomainFindNetByName(vmdef, net->mac, net->ifname) >= 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s already exists."), disk->dst); + return -1; + } + + if (virDomainNetInsert(vmdef, net)) { + virReportOOMError(); + return -1; + } + /* always PCI ? */ + qemuDomainAssignPCIAddresses(vmdef); + newdev->data.net = NULL; + break; default: qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("Sorry, the device is not suppored for now")); @@ -4145,6 +4178,7 @@ static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, { int x; virDomainDiskDefPtr disk; + virDomainNetDefPtr net; switch(device->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -4157,6 +4191,29 @@ static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, } virDomainDiskRemove(vmdef, x); break; + case VIR_DOMAIN_DEVICE_NET: + net = device->data.net; + /* need to find mac address */ + if (net->ifname == NULL && strlen((char*)net->mac) == 0) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("interface mac or name must be specified.")); + return -1; + } + net = device->data.net; + x = qemuDomainFindNetByName(vmdef, net->mac, net->ifname); + if (x < 0) { + if (net->ifname) + qemuReportError(VIR_ERR_INVALID_ARG, + _("interface mac: %s name: %s doesn't exist."), + net->mac, net->ifname); + else + qemuReportError(VIR_ERR_INVALID_ARG, + _("interface mac: %s doesn't exist."), + net->mac); + return -1; + } + virDomainNetRemove(vmdef, x); + break; default: qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("Sorry, the device is not suppored for now")); -- 1.7.1
On Thu, Feb 24, 2011 at 01:08:40PM +0900, KAMEZAWA Hiroyuki wrote:
From b06da6d9b4ee996046af72a81c89b90852372e53 Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa@bluextal.(none)> Date: Thu, 24 Feb 2011 13:08:54 +0900 Subject: [PATCH 2/2] libvirt/qemu : support attach/detach-interface --persistent with qemu
Now, virsh attach/detach-interface have --persistent option for updating inactive domain but it's only supported in Xen.
This patch adds support for qemu.
Changelog v1->v2: - fixed TABs - fixed header file, type of a function
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> --- src/conf/domain_conf.c | 50 ++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 ++ src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b97c1f0..660573b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4924,6 +4924,56 @@ void virDomainDiskRemove(virDomainDefPtr def, size_t i) } }
+int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net)
Function duplicated.
+{ + if (VIR_REALLOC_N(def->nets, def->nnets) < 0) + return -1; + def->nets[def->nnets] = net; + def->nnets++; + return 0; +} + +void virDomainNetRemove(virDomainDefPtr def, size_t i)
Function duplicated.
+{ + 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 virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net)
Function duplicated.
+{ + if (VIR_REALLOC_N(def->nets, def->nnets) < 0) + return -1; + def->nets[def->nnets] = net; + def->nnets++; + return 0; +} + +void virDomainNetRemove(virDomainDefPtr def, size_t i)
Function duplicated.
+{ + 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 virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 30aeccc..396dfbc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1270,6 +1270,9 @@ int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def);
void virDomainDiskRemove(virDomainDefPtr def, size_t i);
+void virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net);
prototype mismatch: int c file this function returns int.
+void virDomainNetRemove(virDomainDefPtr def, size_t i); + int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); void virDomainControllerInsertPreAlloced(virDomainDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 797a670..eef5266 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -245,6 +245,8 @@ virDomainDiskIoTypeToString; virDomainDiskRemove; virDomainDiskTypeFromString; virDomainDiskTypeToString; +virDomainNetInsert; +virDomainNetRemove; virDomainFSDefFree; virDomainFindByID; virDomainFindByName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e75478a..00a240b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4097,6 +4097,22 @@ static int qemuDomainFindDiskByName(virDomainDefPtr vmdef, const char *name) } return -1; } + +static int qemuDomainFindNetByName(virDomainDefPtr vmdef, + const unsigned char *mac, const char *ifname) +{ + virDomainNetDefPtr net; + int i; + + for (i = 0; i < vmdef->nnets; i++) { + net = vmdef->nets[i]; + if (!strcmp((char*)net->mac, (char*)mac)) + return i; + if (ifname && !strcmp((char*)net->ifname, (char*)ifname)) + return i; + } + return -1; +} /* * Attach a device given by XML, the change will be persistent * and domain XML definition file is updated. @@ -4105,6 +4121,7 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr newdev) { virDomainDiskDefPtr disk; + virDomainNetDefPtr net;
/* At first, check device confliction */ switch(newdev->type) { @@ -4131,6 +4148,22 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, } newdev->data.disk = NULL; break; + case VIR_DOMAIN_DEVICE_NET: + net = newdev->data.net; + if (qemuDomainFindNetByName(vmdef, net->mac, net->ifname) >= 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s already exists."), disk->dst);
disk->dst -> net->mac or something else about net device.
+ return -1; + } + + if (virDomainNetInsert(vmdef, net)) { + virReportOOMError(); + return -1; + } + /* always PCI ? */ + qemuDomainAssignPCIAddresses(vmdef); + newdev->data.net = NULL; + break; default: qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("Sorry, the device is not suppored for now")); @@ -4145,6 +4178,7 @@ static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, { int x; virDomainDiskDefPtr disk; + virDomainNetDefPtr net;
switch(device->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -4157,6 +4191,29 @@ static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, } virDomainDiskRemove(vmdef, x); break; + case VIR_DOMAIN_DEVICE_NET: + net = device->data.net; + /* need to find mac address */ + if (net->ifname == NULL && strlen((char*)net->mac) == 0) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("interface mac or name must be specified.")); + return -1; + } + net = device->data.net; + x = qemuDomainFindNetByName(vmdef, net->mac, net->ifname); + if (x < 0) { + if (net->ifname) + qemuReportError(VIR_ERR_INVALID_ARG, + _("interface mac: %s name: %s doesn't exist."), + net->mac, net->ifname); + else + qemuReportError(VIR_ERR_INVALID_ARG, + _("interface mac: %s doesn't exist."), + net->mac); + return -1; + } + virDomainNetRemove(vmdef, x); + break; default: qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("Sorry, the device is not suppored for now")); -- 1.7.1
-- Thanks, Hu Tao
On Thu, 24 Feb 2011 14:47:00 +0800 Hu Tao <hutao@cn.fujitsu.com> wrote:
On Thu, Feb 24, 2011 at 01:08:40PM +0900, KAMEZAWA Hiroyuki wrote:
From b06da6d9b4ee996046af72a81c89b90852372e53 Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa@bluextal.(none)> Date: Thu, 24 Feb 2011 13:08:54 +0900 Subject: [PATCH 2/2] libvirt/qemu : support attach/detach-interface --persistent with qemu
Now, virsh attach/detach-interface have --persistent option for updating inactive domain but it's only supported in Xen.
This patch adds support for qemu.
Changelog v1->v2: - fixed TABs - fixed header file, type of a function
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> --- src/conf/domain_conf.c | 50 ++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 ++ src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b97c1f0..660573b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4924,6 +4924,56 @@ void virDomainDiskRemove(virDomainDefPtr def, size_t i) } }
+int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net)
Function duplicated.
+{ + if (VIR_REALLOC_N(def->nets, def->nnets) < 0) + return -1; + def->nets[def->nnets] = net; + def->nnets++; + return 0; +} + +void virDomainNetRemove(virDomainDefPtr def, size_t i)
Function duplicated.
some bug in my patch(git) handling..
+{ + 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 virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net)
Function duplicated.
+{ + if (VIR_REALLOC_N(def->nets, def->nnets) < 0) + return -1; + def->nets[def->nnets] = net; + def->nnets++; + return 0; +} + +void virDomainNetRemove(virDomainDefPtr def, size_t i)
Function duplicated.
+{ + 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 virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 30aeccc..396dfbc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1270,6 +1270,9 @@ int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def);
void virDomainDiskRemove(virDomainDefPtr def, size_t i);
+void virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net);
prototype mismatch: int c file this function returns int.
I though this fixed but..
+void virDomainNetRemove(virDomainDefPtr def, size_t i); + int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); void virDomainControllerInsertPreAlloced(virDomainDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 797a670..eef5266 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -245,6 +245,8 @@ virDomainDiskIoTypeToString; virDomainDiskRemove; virDomainDiskTypeFromString; virDomainDiskTypeToString; +virDomainNetInsert; +virDomainNetRemove; virDomainFSDefFree; virDomainFindByID; virDomainFindByName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e75478a..00a240b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4097,6 +4097,22 @@ static int qemuDomainFindDiskByName(virDomainDefPtr vmdef, const char *name) } return -1; } + +static int qemuDomainFindNetByName(virDomainDefPtr vmdef, + const unsigned char *mac, const char *ifname) +{ + virDomainNetDefPtr net; + int i; + + for (i = 0; i < vmdef->nnets; i++) { + net = vmdef->nets[i]; + if (!strcmp((char*)net->mac, (char*)mac)) + return i; + if (ifname && !strcmp((char*)net->ifname, (char*)ifname)) + return i; + } + return -1; +} /* * Attach a device given by XML, the change will be persistent * and domain XML definition file is updated. @@ -4105,6 +4121,7 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr newdev) { virDomainDiskDefPtr disk; + virDomainNetDefPtr net;
/* At first, check device confliction */ switch(newdev->type) { @@ -4131,6 +4148,22 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, } newdev->data.disk = NULL; break; + case VIR_DOMAIN_DEVICE_NET: + net = newdev->data.net; + if (qemuDomainFindNetByName(vmdef, net->mac, net->ifname) >= 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s already exists."), disk->dst);
disk->dst -> net->mac or something else about net device.
yes. Thanks, -Kame
On Thu, Feb 24, 2011 at 01:06:36PM +0900, KAMEZAWA Hiroyuki wrote:
From d6c65102224c493f67008b6521b0115d798f5a67 Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa@bluextal.(none)> Date: Thu, 24 Feb 2011 12:14:25 +0900 Subject: [PATCH 1/2] libvirt/qemu - support updating inactive domains.
Now, virsh attach-disk/detach-disk has --persistent option and it can update XML definition of inactive domains. But, it's only supported in Xen.
This patch adds support for attach-disk/detach-disk for qemu.
Changelog v1->v2: - fixed TABs - fixed PCI address assignment for VIRTIO driver.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> --- src/qemu/qemu_driver.c | 168 +++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 158 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8b15a3e..e75478a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4085,14 +4085,164 @@ cleanup: return ret; }
-static int qemudDomainAttachDeviceFlags(virDomainPtr dom, +static int qemuDomainFindDiskByName(virDomainDefPtr vmdef, const char *name) +{ + virDomainDiskDefPtr vdisk; + int i; + + for (i = 0; i < vmdef->ndisks; i++) { + vdisk = vmdef->disks[i]; + if (!strcmp(vdisk->dst, name))
Maybe STREQ() is better here as other libvirt code uses STREQ() rather than strcmp().
+ return i; + } + return -1; +} +/* + * 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) +{ + virDomainDiskDefPtr disk; + + /* At first, check device confliction */ + switch(newdev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = newdev->data.disk; + if (qemuDomainFindDiskByName(vmdef, disk->dst) >= 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s already exists."), disk->dst); + return -1; + } + + if (virDomainDiskInsert(vmdef, disk)) { + virReportOOMError();
I think it's better to report OOM error by virDomainDiskInsert() itself, otherwise every caller has to call virReportOOMError() when fails.
+ return -1; + } + if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { + /* + * Address/Drive information is NULL. If virtio, PCI address + * shoule be fixed. Other devices as IDE, SCSI...will get ID + * automatically + */ + qemuDomainAssignPCIAddresses(vmdef); + + } + newdev->data.disk = NULL; + break; + default: + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("Sorry, the device is not suppored for now")); + return -1; + } + + return 0; +} + +static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, + virDomainDeviceDefPtr device) +{ + int x; + virDomainDiskDefPtr disk; + + switch(device->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = device->data.disk; + x = qemuDomainFindDiskByName(vmdef, disk->dst); + if (x < 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s doesn't exist."), disk->dst); + return -1; + } + virDomainDiskRemove(vmdef, x); + break; + default: + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("Sorry, the device is not suppored for now")); + return -1; + } + return 0; +} + +static int qemuDomainModifyDevicePersistent(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")); + unsigned int attach) +{ + struct qemud_driver *driver; + virDomainDeviceDefPtr device; + virDomainDefPtr vmdef; + virDomainObjPtr vm; + int ret = -1; + + if (!dom || !dom->conn || !dom->name || !xml) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("internal error : %s"), __FUNCTION__); + return -1; + } + + if (dom->conn->flags & VIR_CONNECT_RO) return -1;
No need to check the RO flag here: it should be and is done in virDomainAttachDevice().
+ + driver = dom->conn->privateData; + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, _("cannot find domain '%s'"), + dom->name); + goto unlock_out; + } + + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) { + /* + * 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_INVALID_ARG, + _("cannot update alive domain : %s"), __FUNCTION__);
I think we can silently fail here. Currently this function (qemuDomainModifyDevicePersistent) is called only for modifying inactive domains.
+ goto endjob; + } + vmdef = virDomainObjGetPersistentDef(driver->caps, vm); + + if (!vmdef) + goto endjob; + + device = virDomainDeviceDefParse(driver->caps, + vmdef, xml, VIR_DOMAIN_XML_INACTIVE); + if (!device) + goto endjob; + + if (attach) { + ret = qemuDomainAttachDevicePersistent(vmdef, device); + if (ret < 0) + goto out; + } else { + ret = qemuDomainDetachDevicePersistent(vmdef, device); + if (ret < 0) + goto out; }
This can be refined as: if (attach) ret = qemuDomainAttachDevicePersistent(vmdef, device); else ret = qemuDomainDetachDevicePersistent(vmdef, device); if (ret < 0) goto out;
+ ret = virDomainSaveConfig(driver->configDir, vmdef); + +out: + virDomainDeviceDefFree(device); +endjob: + if (qemuDomainObjEndJob(vm) == 0) + vm = NULL; + if (vm) + virDomainObjUnlock(vm); + /* Note: insert of newdev is done by copy */ +unlock_out: + 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);
return qemudDomainAttachDevice(dom, xml); } @@ -4306,11 +4456,9 @@ 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; - } + + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) + return qemuDomainModifyDevicePersistent(dom, xml, 0);
return qemudDomainDetachDevice(dom, xml); } -- 1.7.1
-- Thanks, Hu Tao
Thank you for review. On Thu, 24 Feb 2011 14:25:24 +0800 Hu Tao <hutao@cn.fujitsu.com> wrote:
On Thu, Feb 24, 2011 at 01:06:36PM +0900, KAMEZAWA Hiroyuki wrote:
From d6c65102224c493f67008b6521b0115d798f5a67 Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa@bluextal.(none)> Date: Thu, 24 Feb 2011 12:14:25 +0900 Subject: [PATCH 1/2] libvirt/qemu - support updating inactive domains.
Now, virsh attach-disk/detach-disk has --persistent option and it can update XML definition of inactive domains. But, it's only supported in Xen.
This patch adds support for attach-disk/detach-disk for qemu.
Changelog v1->v2: - fixed TABs - fixed PCI address assignment for VIRTIO driver.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> --- src/qemu/qemu_driver.c | 168 +++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 158 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8b15a3e..e75478a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4085,14 +4085,164 @@ cleanup: return ret; }
-static int qemudDomainAttachDeviceFlags(virDomainPtr dom, +static int qemuDomainFindDiskByName(virDomainDefPtr vmdef, const char *name) +{ + virDomainDiskDefPtr vdisk; + int i; + + for (i = 0; i < vmdef->ndisks; i++) { + vdisk = vmdef->disks[i]; + if (!strcmp(vdisk->dst, name))
Maybe STREQ() is better here as other libvirt code uses STREQ() rather than strcmp().
Hmm, ok.
+ return i; + } + return -1; +} +/* + * 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) +{ + virDomainDiskDefPtr disk; + + /* At first, check device confliction */ + switch(newdev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = newdev->data.disk; + if (qemuDomainFindDiskByName(vmdef, disk->dst) >= 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s already exists."), disk->dst); + return -1; + } + + if (virDomainDiskInsert(vmdef, disk)) { + virReportOOMError();
I think it's better to report OOM error by virDomainDiskInsert() itself, otherwise every caller has to call virReportOOMError() when fails.
ok, will add a patch for conf/domain_conf.c
+ return -1; + } + if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { + /* + * Address/Drive information is NULL. If virtio, PCI address + * shoule be fixed. Other devices as IDE, SCSI...will get ID + * automatically + */ + qemuDomainAssignPCIAddresses(vmdef); + + } + newdev->data.disk = NULL; + break; + default: + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("Sorry, the device is not suppored for now")); + return -1; + } + + return 0; +} + +static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, + virDomainDeviceDefPtr device) +{ + int x; + virDomainDiskDefPtr disk; + + switch(device->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = device->data.disk; + x = qemuDomainFindDiskByName(vmdef, disk->dst); + if (x < 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s doesn't exist."), disk->dst); + return -1; + } + virDomainDiskRemove(vmdef, x); + break; + default: + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("Sorry, the device is not suppored for now")); + return -1; + } + return 0; +} + +static int qemuDomainModifyDevicePersistent(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")); + unsigned int attach) +{ + struct qemud_driver *driver; + virDomainDeviceDefPtr device; + virDomainDefPtr vmdef; + virDomainObjPtr vm; + int ret = -1; + + if (!dom || !dom->conn || !dom->name || !xml) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("internal error : %s"), __FUNCTION__); + return -1; + } + + if (dom->conn->flags & VIR_CONNECT_RO) return -1;
No need to check the RO flag here: it should be and is done in virDomainAttachDevice().
Ok, will remove.
+ + driver = dom->conn->privateData; + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, _("cannot find domain '%s'"), + dom->name); + goto unlock_out; + } + + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) { + /* + * 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_INVALID_ARG, + _("cannot update alive domain : %s"), __FUNCTION__);
I think we can silently fail here. Currently this function (qemuDomainModifyDevicePersistent) is called only for modifying inactive domains.
It seems...."virDomainObjIsActive(vm) check" is lacked.. Above 'if' returns true only when timeout..
+ goto endjob; + } + vmdef = virDomainObjGetPersistentDef(driver->caps, vm); + + if (!vmdef) + goto endjob; + + device = virDomainDeviceDefParse(driver->caps, + vmdef, xml, VIR_DOMAIN_XML_INACTIVE); + if (!device) + goto endjob; + + if (attach) { + ret = qemuDomainAttachDevicePersistent(vmdef, device); + if (ret < 0) + goto out; + } else { + ret = qemuDomainDetachDevicePersistent(vmdef, device); + if (ret < 0) + goto out; }
This can be refined as:
if (attach) ret = qemuDomainAttachDevicePersistent(vmdef, device); else ret = qemuDomainDetachDevicePersistent(vmdef, device);
if (ret < 0) goto out;
will fix. Thanks, -Kame
+ + driver = dom->conn->privateData; + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, _("cannot find domain '%s'"), + dom->name); + goto unlock_out; + } + + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) { + /* + * 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_INVALID_ARG, + _("cannot update alive domain : %s"), __FUNCTION__);
I think we can silently fail here. Currently this function (qemuDomainModifyDevicePersistent) is called only for modifying inactive domains.
It seems...."virDomainObjIsActive(vm) check" is lacked..
Yes. What about checking for flag VIR_DOMAIN_DEVICE_MODIFY_LIVE? This flag is set if domain is active, for example, see cmdAttachDisk().
Above 'if' returns true only when timeout..
-- Thanks, Hu Tao
On Thu, 24 Feb 2011 15:29:46 +0800 Hu Tao <hutao@cn.fujitsu.com> wrote:
+ + driver = dom->conn->privateData; + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, _("cannot find domain '%s'"), + dom->name); + goto unlock_out; + } + + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) { + /* + * 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_INVALID_ARG, + _("cannot update alive domain : %s"), __FUNCTION__);
I think we can silently fail here. Currently this function (qemuDomainModifyDevicePersistent) is called only for modifying inactive domains.
It seems...."virDomainObjIsActive(vm) check" is lacked..
Yes. What about checking for flag VIR_DOMAIN_DEVICE_MODIFY_LIVE? This flag is set if domain is active, for example, see cmdAttachDisk().
checking virDomainObjIsActive(vm) under lock is required. As you pointed out, if --persistent is specified in virsh attach-disk, == if (vshCommandOptBool(cmd, "persistent")) { flags = VIR_DOMAIN_DEVICE_MODIFY_CONFIG; if (virDomainIsActive(dom) == 1) flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE; ret = virDomainAttachDeviceFlags(dom, buffer, flags); } else { ret = virDomainAttachDevice(dom, buffer); } == Both of MODIFY_CONFIG, MODIFY_LIVE flags are always set if domain is alive. * Attach a virtual device to a domain, using the flags parameter * to control how the device is attached. VIR_DOMAIN_DEVICE_MODIFY_CURRENT * specifies that the device allocation is made based on current domain * state. VIR_DOMAIN_DEVICE_MODIFY_LIVE specifies that the device shall be * allocated to the active domain instance only and is not added to the * persisted domain configuration. VIR_DOMAIN_DEVICE_MODIFY_CONFIG * specifies that the device shall be allocated to the persisted domain * configuration only. Note that the target hypervisor must return an * error if unable to satisfy flags. E.g. the hypervisor driver will * return failure if LIVE is specified but it only supports modifying the * persisted device allocation.
From above, if VIR_DOMAIN_DEVICE_MODIFY_LIVE is set....this function should fail? Ok. But no explanation of API when both of MODIFY_LIVE and MODIFY_CONFIG are set at the same time.
I'll add a test of VIR_DOMAIN_DEVICE_MODIFY_LIVE for now. But I'll remove it later. What we want to know here is the caller wants persistent change or not. I think I'll update this function to allow modification of active domain in future. Thanks, -Kame
On Thu, 24 Feb 2011 16:43:25 +0900 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
On Thu, 24 Feb 2011 15:29:46 +0800 Hu Tao <hutao@cn.fujitsu.com> wrote: e set at the same time.
I'll add a test of VIR_DOMAIN_DEVICE_MODIFY_LIVE for now. But I'll remove it later. What we want to know here is the caller wants persistent change or not.
I think I'll update this function to allow modification of active domain in future.
I'll add this logic in the next post. Thanks, -Kame == + /* + * 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_INVALID_ARG, "%s", + _("Now, cannot modify alive domain and its definition " + "at the same time.")); return -1; } ....... +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 only MODIFY_LIVE, MODIFY_CONFIG are supported now"), + flags); +
participants (2)
-
Hu Tao -
KAMEZAWA Hiroyuki