>From ddcd9cb499dd73dfc91c9f2cc97c064d6dda17fc Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; Date: Wed, 16 Mar 2011 14:05:21 +0900 Subject: [PATCHv5 2/3] libvirt/qemu - support persistent modification of qemu disks support changes of disks by VIR_DOMAIN_DEVICE_MODIFY_CONFIG for qemu. Changelog v4->v5: - moved some functions to domain_conf.c - added virDomainDefAddImplicitControllers() for ide/scsi - report OOM. - clean up. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; --- src/conf/domain_conf.c | 22 ++++++++++++++++++++++ src/conf/domain_conf.h | 3 ++- src/libvirt_private.syms | 2 ++ src/qemu/qemu_driver.c | 29 +++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 16e1291..1d39481 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 30aeccc..320ca13 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1261,7 +1261,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, @@ -1269,6 +1269,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 c88d934..1761dfb 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 dbd5bd3..d6d7ad0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4137,8 +4137,27 @@ cleanup: static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr newdev) { + virDomainDiskDefPtr disk; 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; + } + if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { + if (qemuDomainAssignPCIAddresses(vmdef) < 0) + return -1; + } else if (virDomainDefAddImplicitControllers(vmdef) < 0) + return -1;

+ newdev->data.disk = NULL; + break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Sorry, the device is not supported for now")); @@ -4151,7 +4170,17 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, 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 Mon, 21 Mar 2011 16:05:19 +0800 Wen Congyang <wency(a)cn.fujitsu.com&gt; wrote: > At 03/16/2011 02:45 PM, KAMEZAWA Hiroyuki Write: >> >From ddcd9cb499dd73dfc91c9f2cc97c064d6dda17fc Mon Sep 17 00:00:00 2001 >> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; >> Date: Wed, 16 Mar 2011 14:05:21 +0900 >> Subject: [PATCHv5 2/3] libvirt/qemu - support persistent modification of qemu disks >> >> support changes of disks by VIR_DOMAIN_DEVICE_MODIFY_CONFIG >> for qemu. >> >> Changelog v4->v5: >> - moved some functions to domain_conf.c >> - added virDomainDefAddImplicitControllers() for ide/scsi >> - report OOM. >> - clean up. >> >> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; >> --- >> src/conf/domain_conf.c | 22 ++++++++++++++++++++++ >> src/conf/domain_conf.h | 3 ++- >> src/libvirt_private.syms | 2 ++ >> src/qemu/qemu_driver.c | 29 +++++++++++++++++++++++++++++ >> 4 files changed, 55 insertions(+), 1 deletions(-) >> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 16e1291..1d39481 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 30aeccc..320ca13 100644 >> --- a/src/conf/domain_conf.h >> +++ b/src/conf/domain_conf.h >> @@ -1261,7 +1261,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, >> @@ -1269,6 +1269,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 c88d934..1761dfb 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 dbd5bd3..d6d7ad0 100644 >> --- a/src/qemu/qemu_driver.c >> +++ b/src/qemu/qemu_driver.c >> @@ -4137,8 +4137,27 @@ cleanup: >> static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, >> virDomainDeviceDefPtr newdev) >> { >> + virDomainDiskDefPtr disk; >> >> 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; >> + } >> + if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { >> + if (qemuDomainAssignPCIAddresses(vmdef) < 0) >> + return -1; >> + } else if (virDomainDefAddImplicitControllers(vmdef) < 0) >> + return -1; > > When qemuDomainAssignPCIAddresses() or virDomainDefAddImplicitControllers() failed, > we should remove disk from vmdef, otherwise, it will cause libvirtd crashed. > By returning -1, the vmdef is not saved and no one will see this modified vmdef (because the task is inactive here.) Do I need to care ?

Thanks, -Kame

On Wed, 23 Mar 2011 13:15:39 +0800 Wen Congyang <wency(a)cn.fujitsu.com&gt; wrote: > At 03/23/2011 12:39 PM, KAMEZAWA Hiroyuki Write: >> On Mon, 21 Mar 2011 16:05:19 +0800 >> Wen Congyang <wency(a)cn.fujitsu.com&gt; wrote: >> >>> At 03/16/2011 02:45 PM, KAMEZAWA Hiroyuki Write: >>>> >From ddcd9cb499dd73dfc91c9f2cc97c064d6dda17fc Mon Sep 17 00:00:00 2001 >>>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; >>>> Date: Wed, 16 Mar 2011 14:05:21 +0900 >>>> Subject: [PATCHv5 2/3] libvirt/qemu - support persistent modification of qemu disks >>>> >>>> support changes of disks by VIR_DOMAIN_DEVICE_MODIFY_CONFIG >>>> for qemu. >>>> >>>> Changelog v4->v5: >>>> - moved some functions to domain_conf.c >>>> - added virDomainDefAddImplicitControllers() for ide/scsi >>>> - report OOM. >>>> - clean up. >>>> >>>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; >>>> --- >>>> src/conf/domain_conf.c | 22 ++++++++++++++++++++++ >>>> src/conf/domain_conf.h | 3 ++- >>>> src/libvirt_private.syms | 2 ++ >>>> src/qemu/qemu_driver.c | 29 +++++++++++++++++++++++++++++ >>>> 4 files changed, 55 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >>>> index 16e1291..1d39481 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 30aeccc..320ca13 100644 >>>> --- a/src/conf/domain_conf.h >>>> +++ b/src/conf/domain_conf.h >>>> @@ -1261,7 +1261,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, >>>> @@ -1269,6 +1269,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 c88d934..1761dfb 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 dbd5bd3..d6d7ad0 100644 >>>> --- a/src/qemu/qemu_driver.c >>>> +++ b/src/qemu/qemu_driver.c >>>> @@ -4137,8 +4137,27 @@ cleanup: >>>> static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, >>>> virDomainDeviceDefPtr newdev) >>>> { >>>> + virDomainDiskDefPtr disk; >>>> >>>> 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; >>>> + } >>>> + if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { >>>> + if (qemuDomainAssignPCIAddresses(vmdef) < 0) >>>> + return -1; >>>> + } else if (virDomainDefAddImplicitControllers(vmdef) < 0) >>>> + return -1; >>> >>> When qemuDomainAssignPCIAddresses() or virDomainDefAddImplicitControllers() failed, >>> we should remove disk from vmdef, otherwise, it will cause libvirtd crashed. >>> >> >> By returning -1, the vmdef is not saved and no one will see this modified >> vmdef (because the task is inactive here.) >> >> Do I need to care ? > > Yes, you need to care it. > By returning -1, the vmdef is not saved to disk. But vmdef is hashed, and we may access > it again before libvirtd is restarted. > When stoping libvirtd, we will free vmdef, this will cause disk is double freed. > > Here is the steps to reproduce this program: > # cat device.xml > <disk type='file' device='disk'> > <driver name='qemu' type='qcow2'/> > <source file='/var/lib/libvirt/images/test3.img'/> > <target dev='vdb' bus='virtio'/> > <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> > </disk> > # virsh attach-device vm1 device.xml --persistent > error: Failed to attach device from device.xml > error: An error occurred, but the cause is unknown > > Now, run attach-device again or stop libvirtd. It will cause libvirtd crashed. > Hmm, ok. what I should to unahs and free vmdef ?

Thanks, -Kame

>From d69f7c07571c8adfc7720aab77aa1f103eb6e95b Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; Date: Wed, 16 Mar 2011 15:39:14 +0900 Subject: [PATCHv5 3/3] libvirt/qemu - support support persistent modification of qemu interfaces. support changes of interfaces by VIR_DOMAIN_DEVICE_MODIFY_CONFIG for qemu. Changelog v4->v5 - fixed mac handling. - moved some functions to domain_conf.c - clean up. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; --- 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 1d39481..9e4dea6 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 320ca13..d5154f6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1271,6 +1271,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 1761dfb..cdc63db 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 d6d7ad0..45b1bf0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4138,6 +4138,7 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr newdev) { virDomainDiskDefPtr disk; + virDomainNetDefPtr net; switch(newdev->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -4158,6 +4159,31 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef, return -1; 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; + } + /* 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); + return -1; + } + if (virDomainNetInsert(vmdef, net)) { + virReportOOMError(); + return -1; + } + if (qemuDomainAssignPCIAddresses(vmdef) < 0) + return -1;

+ newdev->data.net = NULL; + break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Sorry, the device is not supported for now")); @@ -4171,6 +4197,7 @@ static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef, virDomainDeviceDefPtr device) { virDomainDiskDefPtr disk; + virDomainNetDefPtr net; switch(device->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -4181,6 +4208,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"));

At 03/16/2011 02:38 PM, KAMEZAWA Hiroyuki Write: > Thank you for review and feedbacks. This is v5 onto the latest git tree. > Any comments are welcome. > == >>From ef85d268a0b553cdb784a1d1916ff8cf171adace Mon Sep 17 00:00:00 2001 > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; > Date: Wed, 16 Mar 2011 11:35:41 +0900 > Subject: [PATCHv5 1/3] libivrt/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. > > 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. > > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; > --- > src/qemu/qemu_driver.c | 141 +++++++++++++++++++++++++++++++++++++++++++---- > 1 files changed, 129 insertions(+), 12 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index dac2bf2..dbd5bd3 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -4130,16 +4130,129 @@ cleanup: > return ret; > } > > -static int qemudDomainAttachDeviceFlags(virDomainPtr dom, > +/* > + * 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, Indentation is not consistent.

> - 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, unsigned int flags) > +{ > + struct qemud_driver *driver; > + virDomainDeviceDefPtr device; > + virDomainDefPtr vmdef; > + virDomainObjPtr vm; > + int ret = -1; > + > + if (!xml) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("internal error")); > return -1; > } xml can not be null, because remoteDomainAttachDevice()/remoteDomainAttachDeviceFlags() will fail on client when xml is null.

> + /* > + * 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", > + _("Now, it's unsupported to update active domain's definition.")); Indentation is not consistent. > + goto endjob; > + } > + > + vmdef = virDomainObjGetPersistentDef(driver->caps, vm); > + > + if (!vmdef) > + goto endjob; > + > + device = virDomainDeviceDefParse(driver->caps, > + vmdef, xml, VIR_DOMAIN_XML_INACTIVE); Indentation is not consistent. > + 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 only MODIFY_LIVE, MODIFY_CONFIG are supported now"), Indentation is not consistent. > + flags); > > - return qemudDomainAttachDevice(dom, xml); > + return -1; > } > > > @@ -4354,13 +4467,17 @@ 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 only MODIFY_LIVE, MODIFY_CONFIG are supported now"), Indentation is not consistent. > + flags); > + return -1; > } > > static int qemudDomainGetAutostart(virDomainPtr dom,