Re: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On 09/16/2014 12:24 AM, John Ferlan wrote: On 09/11/2014 07:43 AM, Ján Tomko wrote: Add the following attributes: csum, gso, guest_tso4, guest_tso6, guest_ecn to the driver element of network interface which control the virtio-net device properties of the same names. --- docs/formatdomain.html.in | 27 docs/schemas/domaincommon.rng | 25 +++ src/conf/domain_conf.c | 81 ++ src/conf/domain_conf.h | 5 ++ .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 + tests/qemuxml2xmltest.c| 1 + 6 files changed, 171 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a2ea758..5b2758a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null lt;model type='virtio'/gt; blt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5'/gt;/b lt;/interfacegt; +lt;interface type='network'gt; + lt;source network='default'/gt; + lt;target dev='vnet2'/gt; + lt;model type='virtio'/gt; + blt;driver csum='off' gso='off' guest_tso4='off' guest_tso6='off' guest_ecn='off'/gt;/b +lt;/interfacegt; This doesn't require a driver name='' value? AFAIK only queues require name='vhost'. lt;/devicesgt; .../pre @@ -3949,6 +3955,27 @@ qemu-kvm -net nic,model=? /dev/null processor, resulting in much higher throughput. span class=sinceSince 1.0.6 (QEMU and KVM only)/span /dd + dtcodecsum/code/dt + dd +The codecsum/code attribute with possible values codeon/code +and codeoff/code controls host-side support for packets with +partial checksum values. +span class=sinceSince 1.2.9 (QEMU only)/spanbr/br/ + +bIn general you should leave this option alone, unless you +are very certain you know what you are doing./b + /dd + dtsegment offloading options/dt + dd +The attributes codegso/code, codeguest_tso4/code, +codeguest_tso6/code, codeguest_ecn/code with possible +values of codeon/code and codeoff/code can be used +to tune segment offloading. +span class=sinceSince 1.2.9 (QEMU only)/spanbr/br/ + +bIn general you should leave this option alone, unless you +are very certain you know what you are doing./b s/this option/these options/ [oh - I'm having a flashback to something similar I had to do at my former employer with their virtualization switch software... these got enabled by some application and caused shall we say significant performance issues, especially for older drivers. That particular software was loaded/started after the virtualization network switch and thus reset what our code had done... Bugger to find as it embedded in some output from some network command that was executed rarely in our vswitch network daemon layer] Anyway, I understand it's desired to not say much about them, but is there any need to say what the defaults are? Furthermore, does one domain's setting affect other domains? I guess my curiosity is more is this a domain function or an interface (host) setting. We may want to indicate that here... Is the difference dependent upon the driver name? This is a per (guest) interface setting, I'm not aware of it affecting other interfaces or domains. Perhaps the 'leave this option alone' was too harsh. I'll add the defaults before pushing/sending another version. Jan I know from my previous experience it had a wider affect, but that code had a very different implementation. The vswitch was separate from the guest as a host process to which guests could configure ports. The vswitch software had the configuration magic to the lower level network driver(s) which is where the tso/cko, etc. were managed in the kernel. The equivalent of 'em0', 'eth0', etc - the physical NIC. A vswitch was tied to a particular physical NIC. So any changes to the pNIC cast a wide net, which is why other software setting TSO/CKO options on the same pNIC our vswitch used after our code disabled it caused all sorts of issues. (Just so you understand why I'm asking the question). + /dd /dl h5a name=elementsNICSTargetOverrideOverriding the target element/a/h5 signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On 09/16/2014 12:30 AM, Eric Blake wrote: On 09/11/2014 05:43 AM, Ján Tomko wrote: Add the following attributes: csum, gso, guest_tso4, guest_tso6, guest_ecn to the driver element of network interface which control the virtio-net device properties of the same names. --- docs/formatdomain.html.in | 27 docs/schemas/domaincommon.rng | 25 +++ src/conf/domain_conf.c | 81 ++ src/conf/domain_conf.h | 5 ++ .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 + tests/qemuxml2xmltest.c| 1 + 6 files changed, 171 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a2ea758..5b2758a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null lt;model type='virtio'/gt; blt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5'/gt;/b lt;/interfacegt; +lt;interface type='network'gt; + lt;source network='default'/gt; + lt;target dev='vnet2'/gt; + lt;model type='virtio'/gt; + blt;driver csum='off' gso='off' guest_tso4='off' guest_tso6='off' guest_ecn='off'/gt;/b +lt;/interfacegt; Are we stuck with names with underscores in our XML? I'm still not sure if we've come up with the best naming for exposing all these knobs. I'd rather not mix underscores (event_idx) and other word separators in the same element. Alternatively, we could do something like: driver csum='off' gso='off' guest tso4='off' tso6='off' ecn='off'/ /driver to get rid of the multi-word attributes, but I like the underscores better because they match QEMU arguments. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] qemu: Add support for multiple versions of 'pseries' machine type
On 09/15/2014 07:46 PM, Ján Tomko wrote: On 09/13/2014 05:28 PM, Pradipta Kr. Banerjee wrote: qemu: Add support for multiple versions of 'pseries' machine type qemu for IBM Power processor architecture is adding functionality for supporting multiple 'pseries' machine type versions, each with different capabilities. This patch is for supporting the same Signed-off-by: Pradipta Kr. Banerjee bpra...@in.ibm.com --- src/qemu/qemu_command.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) ACK. Would you like to add a qemuxml2argvtest for 'pseries-2.1' machine with these devices? As of now there are no changes to qemu command line invocation based on different machine type. If required for this patch I'll go ahead and add the test cases for pseries-2.1 and pseries-2.2. pseries-2.2 is the latest which aliases to pseries. Please do let me know your thoughts. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Regards, Pradipta Kumar B(bpra...@in.ibm.com) IBM Systems Technology Labs, India. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] qemu: Add support for multiple versions of 'pseries' machine type
On 09/17/2014 11:02 AM, Pradipta Kumar Banerjee wrote: On 09/15/2014 07:46 PM, Ján Tomko wrote: On 09/13/2014 05:28 PM, Pradipta Kr. Banerjee wrote: qemu: Add support for multiple versions of 'pseries' machine type qemu for IBM Power processor architecture is adding functionality for supporting multiple 'pseries' machine type versions, each with different capabilities. This patch is for supporting the same Signed-off-by: Pradipta Kr. Banerjee bpra...@in.ibm.com --- src/qemu/qemu_command.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) ACK. Would you like to add a qemuxml2argvtest for 'pseries-2.1' machine with these devices? As of now there are no changes to qemu command line invocation based on different machine type. If required for this patch I'll go ahead and add the test cases for pseries-2.1 and pseries-2.2. It is not required for the patch, I just thought a test would help to make sure that we don't break the old machine types if we ever change something in the code. I've pushed the patch now. pseries-2.2 is the latest which aliases to pseries. Please do let me know your thoughts. Oh, so it's the other way than on x86_64 where 'pc' is the alias for the latest pc-i440fx machine (pc-i440fx-2.2 as of now). Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix crash with shared disks
Commit f36a94f introduced a double free on all success paths in qemuSharedDeviceEntryInsert. Only call qemuSharedDeviceEntryFree on the error path and set entry to NULL before jumping there if the entry already is in the hash table. https://bugzilla.redhat.com/show_bug.cgi?id=1142722 --- src/qemu/qemu_conf.c | 26 -- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index ac10b64..adc6caf 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1011,38 +1011,36 @@ qemuSharedDeviceEntryInsert(virQEMUDriverPtr driver, const char *name) { qemuSharedDeviceEntry *entry = NULL; -int ret = -1; if ((entry = virHashLookup(driver-sharedDevices, key))) { /* Nothing to do if the shared scsi host device is already * recorded in the table. */ -if (qemuSharedDeviceEntryDomainExists(entry, name, NULL)) { -ret = 0; -goto cleanup; +if (!qemuSharedDeviceEntryDomainExists(entry, name, NULL)) { +if (VIR_EXPAND_N(entry-domains, entry-ref, 1) 0 || +VIR_STRDUP(entry-domains[entry-ref - 1], name) 0) { +/* entry is owned by the hash table here */ +entry = NULL; +goto error; +} } - -if (VIR_EXPAND_N(entry-domains, entry-ref, 1) 0 || -VIR_STRDUP(entry-domains[entry-ref - 1], name) 0) -goto cleanup; } else { if (VIR_ALLOC(entry) 0 || VIR_ALLOC_N(entry-domains, 1) 0 || VIR_STRDUP(entry-domains[0], name) 0) -goto cleanup; +goto error; entry-ref = 1; if (virHashAddEntry(driver-sharedDevices, key, entry)) -goto cleanup; +goto error; } -ret = 0; +return 0; - cleanup: + error: qemuSharedDeviceEntryFree(entry, NULL); - -return ret; +return -1; } -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domaincaps: Expose UEFI binary path, if it exists
Hi Cole, I'm not subscribed to the list; please CC me on UEFI-related patches. Michal pinged me to review this one. Some comments: On 09/17/14 01:52, Cole Robinson wrote: Check to see if the UEFI binary mentioned in qemu.conf actually exists, and if so expose it in domcapabilities like loader ... value/path/to/ovmf/value /loader We introduce some generic domcaps infrastructure for handling a dynamic list of string values, it may be of use for future bits. --- docs/formatdomaincaps.html.in | 6 docs/schemas/domaincaps.rng| 17 ++--- src/conf/domain_capabilities.c | 23 src/conf/domain_capabilities.h | 8 + src/qemu/qemu_driver.c | 24 + tests/domaincapsschemadata/domaincaps-full.xml | 1 + tests/domaincapstest.c | 49 +- 7 files changed, 115 insertions(+), 13 deletions(-) diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in index 34d746d..6959dfe 100644 --- a/docs/formatdomaincaps.html.in +++ b/docs/formatdomaincaps.html.in @@ -105,6 +105,7 @@ ... lt;os supported='yes'gt; lt;loader supported='yes'gt; + lt;valuegt;/usr/share/OVMF/OVMF_CODE.fdlt;/valuegt; lt;enum name='type'gt; lt;valuegt;romlt;/valuegt; lt;valuegt;pflashlt;/valuegt; @@ -122,6 +123,11 @@ pFor the codeloader/code element, the following can occur:/p dl + dtvalue/dt + ddList of known loader paths. Currently this is only used + to advertise known locations of OVMF binaries for qemu. Binaries + will only be listed if they actually exist on disk./dd + dttype/dt ddWhether loader is a typical BIOS (coderom/code) or an UEFI binary (codepflash/code). This refers to diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng index ad8d966..dfdb9b9 100644 --- a/docs/schemas/domaincaps.rng +++ b/docs/schemas/domaincaps.rng @@ -46,6 +46,9 @@ define name='loader' element name='loader' + optional +ref name='value'/ + /optional ref name='supported'/ ref name='enum'/ /element @@ -85,6 +88,14 @@ /element /define + define name='value' +zeroOrMore + element name='value' +text/ + /element +/zeroOrMore + /define + define name='supported' attribute name='supported' choice @@ -100,11 +111,7 @@ attribute name='name' text/ /attribute -zeroOrMore - element name='value' -text/ - /element -/zeroOrMore +ref name='value'/ /element /zeroOrMore /define diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 5a3c8e7..44e422a 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -46,6 +46,15 @@ static int virDomainCapsOnceInit(void) VIR_ONCE_GLOBAL_INIT(virDomainCaps) +static void +virDomainCapsValuesFree(virDomainCapsValuesPtr values) +{ +size_t i; + +for (i = 0; i values-nvalues; i++) { +VIR_FREE(values-values[i]); +} +} static void virDomainCapsDispose(void *obj) @@ -54,6 +63,8 @@ virDomainCapsDispose(void *obj) VIR_FREE(caps-path); VIR_FREE(caps-machine); + +virDomainCapsValuesFree(caps-os.loader.values); } (1) This leaks the caps-os.loader.values.values array. (Which is a dynamically allocated array of pointers.) virDomainCapsValuesFree() should VIR_FREE() it too. @@ -156,6 +167,17 @@ virDomainCapsEnumFormat(virBufferPtr buf, return ret; } +static void +virDomainCapsValuesFormat(virBufferPtr buf, + virDomainCapsValuesPtr values) +{ +size_t i; + +for (i = 0; i values-nvalues; i++) { +virBufferAsprintf(buf, value%s/value\n, values-values[i]); +} +} (2) virBufferEscapeString() would probably useful here; the filename shouldn't be plainly embedded into an XML fragment. I'm not sure if we paid attention to this with the nvram patches... Hm yes; I rechecked Michal's commits now, and they seem to use virBufferEscapeString(). + #define FORMAT_PROLOGUE(item) \ do {\ virBufferAsprintf(buf, #item supported='%s'%s\n, \ @@ -185,6 +207,7 @@ virDomainCapsLoaderFormat(virBufferPtr buf, { FORMAT_PROLOGUE(loader); +virDomainCapsValuesFormat(buf, loader-values); ENUM_PROCESS(loader, type, virDomainLoaderTypeToString); ENUM_PROCESS(loader, readonly, virTristateBoolTypeToString); diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index 768646b..3d5aaa3 100644 --- a/src/conf/domain_capabilities.h +++
Re: [libvirt] [PATCH] domaincaps: Expose UEFI binary path, if it exists
On 17.09.2014 01:52, Cole Robinson wrote: Check to see if the UEFI binary mentioned in qemu.conf actually exists, and if so expose it in domcapabilities like loader ... value/path/to/ovmf/value /loader We introduce some generic domcaps infrastructure for handling a dynamic list of string values, it may be of use for future bits. --- docs/formatdomaincaps.html.in | 6 docs/schemas/domaincaps.rng| 17 ++--- src/conf/domain_capabilities.c | 23 src/conf/domain_capabilities.h | 8 + src/qemu/qemu_driver.c | 24 + tests/domaincapsschemadata/domaincaps-full.xml | 1 + tests/domaincapstest.c | 49 +- 7 files changed, 115 insertions(+), 13 deletions(-) diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in index 34d746d..6959dfe 100644 --- a/docs/formatdomaincaps.html.in +++ b/docs/formatdomaincaps.html.in @@ -105,6 +105,7 @@ ... lt;os supported='yes'gt; lt;loader supported='yes'gt; + lt;valuegt;/usr/share/OVMF/OVMF_CODE.fdlt;/valuegt; lt;enum name='type'gt; lt;valuegt;romlt;/valuegt; lt;valuegt;pflashlt;/valuegt; @@ -122,6 +123,11 @@ pFor the codeloader/code element, the following can occur:/p dl + dtvalue/dt + ddList of known loader paths. Currently this is only used + to advertise known locations of OVMF binaries for qemu. Binaries + will only be listed if they actually exist on disk./dd + dttype/dt ddWhether loader is a typical BIOS (coderom/code) or an UEFI binary (codepflash/code). This refers to diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng index ad8d966..dfdb9b9 100644 --- a/docs/schemas/domaincaps.rng +++ b/docs/schemas/domaincaps.rng @@ -46,6 +46,9 @@ define name='loader' element name='loader' + optional +ref name='value'/ + /optional ref name='supported'/ I know it doesn't really matter, but I prefer attributes to be defined before any child elements. So I'd move 'supported' a few lines up. ref name='enum'/ /element @@ -85,6 +88,14 @@ /element /define + define name='value' +zeroOrMore + element name='value' +text/ + /element +/zeroOrMore + /define + define name='supported' attribute name='supported' choice @@ -100,11 +111,7 @@ attribute name='name' text/ /attribute -zeroOrMore - element name='value' -text/ - /element -/zeroOrMore +ref name='value'/ /element /zeroOrMore /define diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 5a3c8e7..44e422a 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -46,6 +46,15 @@ static int virDomainCapsOnceInit(void) VIR_ONCE_GLOBAL_INIT(virDomainCaps) +static void +virDomainCapsValuesFree(virDomainCapsValuesPtr values) +{ +size_t i; + +for (i = 0; i values-nvalues; i++) { +VIR_FREE(values-values[i]); +} +} static void virDomainCapsDispose(void *obj) @@ -54,6 +63,8 @@ virDomainCapsDispose(void *obj) VIR_FREE(caps-path); VIR_FREE(caps-machine); + +virDomainCapsValuesFree(caps-os.loader.values); } @@ -156,6 +167,17 @@ virDomainCapsEnumFormat(virBufferPtr buf, return ret; } +static void +virDomainCapsValuesFormat(virBufferPtr buf, + virDomainCapsValuesPtr values) +{ +size_t i; + +for (i = 0; i values-nvalues; i++) { +virBufferAsprintf(buf, value%s/value\n, values-values[i]); +} +} + #define FORMAT_PROLOGUE(item) \ do {\ virBufferAsprintf(buf, #item supported='%s'%s\n, \ @@ -185,6 +207,7 @@ virDomainCapsLoaderFormat(virBufferPtr buf, { FORMAT_PROLOGUE(loader); +virDomainCapsValuesFormat(buf, loader-values); ENUM_PROCESS(loader, type, virDomainLoaderTypeToString); ENUM_PROCESS(loader, readonly, virTristateBoolTypeToString); diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index 768646b..3d5aaa3 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -37,6 +37,13 @@ struct _virDomainCapsEnum { unsigned int values; /* Bitmask of values supported in the corresponding enum */ }; +typedef struct _virDomainCapsValues virDomainCapsValues; +typedef virDomainCapsValues *virDomainCapsValuesPtr; +struct _virDomainCapsValues { +char **values; /* raw string values */ +size_t nvalues; /* number of strings */ +}; While this works, I'd rename this to virDomainCapsStringValues so that it's clear what values do we have in mind. Moreover, if we ever
[libvirt] [PATCH v2 0/2] Expose UEFI binary path
This is practically reworked v1 from Cole. Cole Robinson (1): domaincaps: Expose UEFI binary path, if it exists Michal Privoznik (1): qemu_capabilities: Change virQEMUCapsFillDomainCaps signature docs/formatdomaincaps.html.in | 6 ++ docs/schemas/domaincaps.rng| 17 -- src/conf/domain_capabilities.c | 29 ++ src/conf/domain_capabilities.h | 8 +++ src/qemu/qemu_capabilities.c | 53 + src/qemu/qemu_capabilities.h | 9 ++- src/qemu/qemu_driver.c | 7 ++- tests/domaincapsschemadata/domaincaps-full.xml | 2 + .../domaincaps-qemu_1.6.50-1.xml | 1 + tests/domaincapstest.c | 66 ++ 10 files changed, 167 insertions(+), 31 deletions(-) -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/2] qemu_capabilities: Change virQEMUCapsFillDomainCaps signature
Up till now the virQEMUCapsFillDomainCaps() was type of void as there was no way for it to fail. This is, however, going to change in the next commit. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_capabilities.c | 25 - src/qemu/qemu_capabilities.h | 4 ++-- src/qemu/qemu_driver.c | 3 ++- tests/domaincapstest.c | 19 --- 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9f8868d..c306899 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3608,7 +3608,7 @@ virQEMUCapsGetDefaultMachine(virQEMUCapsPtr qemuCaps) } -static void +static int virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps, virDomainCapsLoaderPtr loader, virArch arch) @@ -3629,10 +3629,11 @@ virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_CAPS_ENUM_SET(loader-readonly, VIR_TRISTATE_BOOL_YES, VIR_TRISTATE_BOOL_NO); +return 0; } -static void +static int virQEMUCapsFillDomainOSCaps(virQEMUCapsPtr qemuCaps, virDomainCapsOSPtr os, virArch arch) @@ -3640,11 +3641,13 @@ virQEMUCapsFillDomainOSCaps(virQEMUCapsPtr qemuCaps, virDomainCapsLoaderPtr loader = os-loader; os-device.supported = true; -virQEMUCapsFillDomainLoaderCaps(qemuCaps, loader, arch); +if (virQEMUCapsFillDomainLoaderCaps(qemuCaps, loader, arch) 0) +return -1; +return 0; } -static void +static int virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, virDomainCapsDeviceDiskPtr disk) { @@ -3667,10 +3670,11 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) VIR_DOMAIN_CAPS_ENUM_SET(disk-bus, VIR_DOMAIN_DISK_BUS_USB); +return 0; } -static void +static int virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps, virDomainCapsDeviceHostdevPtr hostdev) { @@ -3715,10 +3719,11 @@ virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM); } +return 0; } -void +int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, virQEMUCapsPtr qemuCaps) { @@ -3729,7 +3734,9 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, domCaps-maxvcpus = maxvcpus; -virQEMUCapsFillDomainOSCaps(qemuCaps, os, domCaps-arch); -virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, disk); -virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev); +if (virQEMUCapsFillDomainOSCaps(qemuCaps, os, domCaps-arch) 0 || +virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, disk) 0 || +virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) 0) +return -1; +return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0980c00..828bba3 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -324,7 +324,7 @@ int virQEMUCapsInitGuestFromBinary(virCapsPtr caps, virQEMUCapsPtr kvmbinCaps, virArch guestarch); -void virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, - virQEMUCapsPtr qemuCaps); +int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, + virQEMUCapsPtr qemuCaps); #endif /* __QEMU_CAPABILITIES_H__*/ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 15ad64d..4fe5909 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17357,7 +17357,8 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, if (!(domCaps = virDomainCapsNew(emulatorbin, machine, arch, virttype))) goto cleanup; -virQEMUCapsFillDomainCaps(domCaps, qemuCaps); +if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps) 0) +goto cleanup; ret = virDomainCapsFormat(domCaps); cleanup: diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index f240643..0c4b09f 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -28,13 +28,13 @@ #define VIR_FROM_THIS VIR_FROM_NONE -typedef void (*virDomainCapsFill)(virDomainCapsPtr domCaps, - void *opaque); +typedef int (*virDomainCapsFill)(virDomainCapsPtr domCaps, + void *opaque); #define SET_ALL_BITS(x) \ memset((x.values), 0xff, sizeof(x.values)) -static void +static int fillAll(virDomainCapsPtr domCaps, void *opaque ATTRIBUTE_UNUSED) { @@ -60,18 +60,20 @@ fillAll(virDomainCapsPtr domCaps,
[libvirt] [PATCH v2 2/2] domaincaps: Expose UEFI binary path, if it exists
From: Cole Robinson crobi...@redhat.com Check to see if the UEFI binary mentioned in qemu.conf actually exists, and if so expose it in domcapabilities like loader ... value/path/to/ovmf/value /loader We introduce some generic domcaps infrastructure for handling a dynamic list of string values, it may be of use for future bits. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- docs/formatdomaincaps.html.in | 6 +++ docs/schemas/domaincaps.rng| 17 +--- src/conf/domain_capabilities.c | 29 + src/conf/domain_capabilities.h | 8 src/qemu/qemu_capabilities.c | 32 +++--- src/qemu/qemu_capabilities.h | 7 +++- src/qemu/qemu_driver.c | 6 ++- tests/domaincapsschemadata/domaincaps-full.xml | 2 + .../domaincaps-qemu_1.6.50-1.xml | 1 + tests/domaincapstest.c | 49 +++--- 10 files changed, 140 insertions(+), 17 deletions(-) diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in index 34d746d..6959dfe 100644 --- a/docs/formatdomaincaps.html.in +++ b/docs/formatdomaincaps.html.in @@ -105,6 +105,7 @@ ... lt;os supported='yes'gt; lt;loader supported='yes'gt; + lt;valuegt;/usr/share/OVMF/OVMF_CODE.fdlt;/valuegt; lt;enum name='type'gt; lt;valuegt;romlt;/valuegt; lt;valuegt;pflashlt;/valuegt; @@ -122,6 +123,11 @@ pFor the codeloader/code element, the following can occur:/p dl + dtvalue/dt + ddList of known loader paths. Currently this is only used + to advertise known locations of OVMF binaries for qemu. Binaries + will only be listed if they actually exist on disk./dd + dttype/dt ddWhether loader is a typical BIOS (coderom/code) or an UEFI binary (codepflash/code). This refers to diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng index ad8d966..f4a555f 100644 --- a/docs/schemas/domaincaps.rng +++ b/docs/schemas/domaincaps.rng @@ -47,6 +47,9 @@ define name='loader' element name='loader' ref name='supported'/ + optional +ref name='value'/ + /optional ref name='enum'/ /element /define @@ -85,6 +88,14 @@ /element /define + define name='value' +zeroOrMore + element name='value' +text/ + /element +/zeroOrMore + /define + define name='supported' attribute name='supported' choice @@ -100,11 +111,7 @@ attribute name='name' text/ /attribute -zeroOrMore - element name='value' -text/ - /element -/zeroOrMore +ref name='value'/ /element /zeroOrMore /define diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 5a3c8e7..7c59912 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -48,12 +48,28 @@ VIR_ONCE_GLOBAL_INIT(virDomainCaps) static void +virDomainCapsStringValuesFree(virDomainCapsStringValuesPtr values) +{ +size_t i; + +if (!values || !values-values) +return; + +for (i = 0; i values-nvalues; i++) +VIR_FREE(values-values[i]); +VIR_FREE(values-values); +} + + +static void virDomainCapsDispose(void *obj) { virDomainCapsPtr caps = obj; VIR_FREE(caps-path); VIR_FREE(caps-machine); + +virDomainCapsStringValuesFree(caps-os.loader.values); } @@ -156,6 +172,18 @@ virDomainCapsEnumFormat(virBufferPtr buf, return ret; } + +static void +virDomainCapsStringValuesFormat(virBufferPtr buf, +virDomainCapsStringValuesPtr values) +{ +size_t i; + +for (i = 0; i values-nvalues; i++) +virBufferEscapeString(buf, value%s/value\n, values-values[i]); +} + + #define FORMAT_PROLOGUE(item) \ do {\ virBufferAsprintf(buf, #item supported='%s'%s\n, \ @@ -185,6 +213,7 @@ virDomainCapsLoaderFormat(virBufferPtr buf, { FORMAT_PROLOGUE(loader); +virDomainCapsStringValuesFormat(buf, loader-values); ENUM_PROCESS(loader, type, virDomainLoaderTypeToString); ENUM_PROCESS(loader, readonly, virTristateBoolTypeToString); diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index 768646b..597ac75 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -37,6 +37,13 @@ struct _virDomainCapsEnum { unsigned int values; /* Bitmask of values supported in the corresponding enum */ }; +typedef struct _virDomainCapsStringValues virDomainCapsStringValues; +typedef virDomainCapsStringValues *virDomainCapsStringValuesPtr; +struct _virDomainCapsStringValues { +char **values; /* raw string values */ +
Re: [libvirt] [PATCH v2 1/2] qemu_capabilities: Change virQEMUCapsFillDomainCaps signature
On 09/17/14 14:15, Michal Privoznik wrote: Up till now the virQEMUCapsFillDomainCaps() was type of void as there was no way for it to fail. This is, however, going to change in the next commit. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_capabilities.c | 25 - src/qemu/qemu_capabilities.h | 4 ++-- src/qemu/qemu_driver.c | 3 ++- tests/domaincapstest.c | 19 --- 4 files changed, 32 insertions(+), 19 deletions(-) Seems reasonable. Acked-by: Laszlo Ersek ler...@redhat.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/2] domaincaps: Expose UEFI binary path, if it exists
On 09/17/14 14:15, Michal Privoznik wrote: diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 0c4b09f..8543963 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -34,6 +34,27 @@ typedef int (*virDomainCapsFill)(virDomainCapsPtr domCaps, #define SET_ALL_BITS(x) \ memset((x.values), 0xff, sizeof(x.values)) +static int ATTRIBUTE_SENTINEL +fillStringValues(virDomainCapsStringValuesPtr values, ...) +{ +int ret = 0; +va_list list; +const char *str; + +va_start(list, values); +while ((str = va_arg(list, const char *))) { +if (VIR_REALLOC_N(values-values, values-nvalues + 1) 0 || +VIR_STRDUP(values-values[values-nvalues], str) 0) { +ret = -1; +break; +} +values-nvalues++; +} +va_end(list); + +return ret; +} Okay, you increment values-nvalues only after. The rest too looks good to me. Acked-by: Laszlo Ersek ler...@redhat.com Thanks Laszlo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] qemu_capabilities: Change virQEMUCapsFillDomainCaps signature
On 09/17/2014 08:15 AM, Michal Privoznik wrote: Up till now the virQEMUCapsFillDomainCaps() was type of void as there was no way for it to fail. This is, however, going to change in the next commit. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_capabilities.c | 25 - src/qemu/qemu_capabilities.h | 4 ++-- src/qemu/qemu_driver.c | 3 ++- tests/domaincapstest.c | 19 --- 4 files changed, 32 insertions(+), 19 deletions(-) ACK - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/2] domaincaps: Expose UEFI binary path, if it exists
On 09/17/2014 08:15 AM, Michal Privoznik wrote: From: Cole Robinson crobi...@redhat.com Check to see if the UEFI binary mentioned in qemu.conf actually exists, and if so expose it in domcapabilities like loader ... value/path/to/ovmf/value /loader We introduce some generic domcaps infrastructure for handling a dynamic list of string values, it may be of use for future bits. Signed-off-by: Michal Privoznik mpriv...@redhat.com ACK, thanks for cleaning it up. But please change authorship since the patch is reasonably altered, I won't be offended :) - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] rpc: make daemon spawning a bit more intelligent
On Tue, Sep 16, 2014 at 10:22:53AM -0400, John Ferlan wrote: On 09/16/2014 05:16 AM, Martin Kletzander wrote: This way it behaves more like the daemon itself does (acquiring a pidfile, deleting the socket before binding, etc.). Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138604 Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/rpc/virnetsocket.c | 65 +++--- 1 file changed, 57 insertions(+), 8 deletions(-) The error path/retry logic needs a tweak... I added some inline thinking since we don't have a virtual whiteboard to share on this! Yes, it does. I didn't think it through from scratch, just adjusted to your comments. This time I went through it few times. Just let me confirm I understood what you meant everywhere. diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 80aeddf..7be1492 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -51,9 +51,11 @@ #include virlog.h #include virfile.h #include virthread.h +#include virpidfile.h #include virprobe.h #include virprocess.h #include virstring.h +#include dirname.h #include passfd.h #if WITH_SSH2 @@ -541,7 +543,10 @@ int virNetSocketNewConnectUNIX(const char *path, const char *binary, virNetSocketPtr *retsock) { +char *binname = NULL; +char *pidpath = NULL; int fd, passfd = -1; +int pidfd = -1; virSocketAddr localAddr; virSocketAddr remoteAddr; @@ -580,16 +585,47 @@ int virNetSocketNewConnectUNIX(const char *path, goto error; } +if (!(binname = last_component(binary)) || binname[0] == '\0') { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Cannot determine basename for binary '%s'), + binary); +goto error; +} + +if (virPidFileConstructPath(false, NULL, binname, pidpath) 0) +goto error; Since the first param is false, we are guaranteed only that 'pidpath' is the path to the virGetUserRuntimeDirectory() for $binname.pid. We are not sure if we created the path in virFileMakePathHelper() or not. If we later then delete the file on the error path how does that affect the daemon that wins the race? See the conundrum? This is only about deleting the pidfile, right? Deleting it only when it is acquired (pidfd = 0) should fix this. I'll try describing it here a little bit more: virNetSocketNewConnectUNIX() is called with (spawnDaemon == true) only if (privileged == false). virPidFileConstructPath() is called also only when (spawnDaemon == true) and guarantees that the path for the pidfile exists and is constructed the same way it is in daemon. The path should not be deleted no matter whether we fail or not, those directories should be kept there for later. + +pidfd = virPidFileAcquirePath(pidpath, false, getpid()); +VIR_FREE(pidpath); Because you VIR_FREE() here, there is no way for the error: path to have a non NULL pidpath... and delete the pidpath. Using VIR_FREE(pidpath); in both error path and before return 0 (my initial idea) should take care of this. +if (pidfd 0) { Getting here means we were unable to get the pidfile lock and I don't think we want to delete the pidpath... Since it's probably owned by some other daemon if (pidfd 0) then it means it will not get deleted. By the way it doesn't have to be deleted, closing should be enough to unlock the file. +/* + * This can happen in a very rare case of two clients spawning two + * daemons at the same time, and the error in the logs that gets + * reset here can be a clue to some future debugging. + */ +virResetLastError(); +spawnDaemon = false; +goto retry; +} If we get here, we've written our pid into the pidfile and we have have the lock... So that means we should own the file. Errors from here should delete the file. Right, there's nothing wrong. + if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) { virReportSystemError(errno, %s, _(Failed to create socket)); goto error; } /* - * We have to fork() here, because umask() is set - * per-process, chmod() is racy and fchmod() has undefined - * behaviour on sockets according to POSIX, so it doesn't - * work outside Linux. + * We already even acquired the pidfile, so no one else should be using + * @path right now. So we're OK to unlink it and paying attention to + * the return value makes no real sense here. Only if it's not an + * abstract socket, of course. + */ +if (path[0] != '@') +unlink(path); + +/* + * We have to fork() here,
Re: [libvirt] [PATCH v2 2/2] domaincaps: Expose UEFI binary path, if it exists
On 17.09.2014 14:57, Cole Robinson wrote: On 09/17/2014 08:15 AM, Michal Privoznik wrote: From: Cole Robinson crobi...@redhat.com Check to see if the UEFI binary mentioned in qemu.conf actually exists, and if so expose it in domcapabilities like loader ... value/path/to/ovmf/value /loader We introduce some generic domcaps infrastructure for handling a dynamic list of string values, it may be of use for future bits. Signed-off-by: Michal Privoznik mpriv...@redhat.com ACK, thanks for cleaning it up. But please change authorship since the patch is reasonably altered, I won't be offended :) - Cole Thanks. Fixed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/n] dumpxml: prepare to output block info
This patch adds the common code for outputting drive sizing information, when a user has requested the new API flag from the previous patch. * src/util/virstoragefile.h (_virStorageSource): Add physical, to mirror virDomainBlockInfo. * docs/schemas/domaincommon.rng (storageSourceExtra): New define. * src/conf/domain_conf.c (virDomainDiskDefFormat): Output sizing when flag is set. (DUMPXML_FLAGS): Add new flag. Signed-off-by: Eric Blake ebl...@redhat.com --- docs/schemas/domaincommon.rng | 22 ++ src/conf/domain_conf.c| 16 +++- src/util/virstoragefile.h | 3 ++- 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c600f22..dd874fc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1292,6 +1292,28 @@ /element /define + define name='storageSourceExtra' combine='choice' +!-- Override of storagecommon.rng to allow domain disks to list + extras without impacting snapshot disks -- +interleave + optional +element name=capacity + ref name=scaledInteger/ +/element + /optional + optional +element name=allocation + ref name=scaledInteger/ +/element + /optional + optional +element name=physical + ref name=scaledInteger/ +/element + /optional +/interleave + /define + define name=diskBackingChain choice ref name=diskBackingStore/ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3ccec1c..3b54619 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -108,7 +108,8 @@ typedef enum { (VIR_DOMAIN_XML_SECURE |\ VIR_DOMAIN_XML_INACTIVE | \ VIR_DOMAIN_XML_UPDATE_CPU |\ - VIR_DOMAIN_XML_MIGRATABLE) + VIR_DOMAIN_XML_MIGRATABLE | \ + VIR_DOMAIN_XML_BLOCK_INFO) verify(((VIR_DOMAIN_XML_INTERNAL_STATUS | VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET | @@ -15887,6 +15888,19 @@ virDomainDiskDefFormat(virBufferPtr buf, flags) 0) return -1; +if (flags VIR_DOMAIN_XML_BLOCK_INFO) { +if (def-src-capacity) +virBufferAsprintf(buf, capacity unit='bytes'%llu/capacity\n, + def-src-capacity); +if (def-src-allocation) +virBufferAsprintf(buf, + allocation unit='bytes'%llu/allocation\n, + def-src-allocation); +if (def-src-physical) +virBufferAsprintf(buf, physical unit='bytes'%llu/physical\n, + def-src-physical); +} + /* Don't format backingStore to inactive XMLs until the code for * persistent storage of backing chains is ready. */ if (!(flags VIR_DOMAIN_XML_INACTIVE) diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 2583e10..681e50a 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -252,8 +252,9 @@ struct _virStorageSource { virStoragePermsPtr perms; virStorageTimestampsPtr timestamps; -unsigned long long allocation; /* in bytes, 0 if unknown */ unsigned long long capacity; /* in bytes, 0 if unknown */ +unsigned long long allocation; /* in bytes, 0 if unknown */ +unsigned long long physical; /* in bytes, 0 if unknown */ size_t nseclabels; virSecurityDeviceLabelDefPtr *seclabels; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC: PATCH 0/2] Display allocation during dumpxml
I'm still working on code to populate the latest numbers for each disk of a domain, including getting numbers for offline domains, but have confirmed that with these two patches alone I'm able to see capacity and allocation numbers for block volumes of live domains (thanks to how we populate backing chain information). So while there are more patches to come, I'd like to get review started on my proposed API addition. Eric Blake (2): dumpxml: add flag to virDomainGetXMLDesc dumpxml: prepare to output block info docs/schemas/domaincommon.rng | 22 ++ include/libvirt/libvirt.h.in | 1 + src/conf/domain_conf.c| 16 +++- src/libvirt.c | 15 +++ src/util/virstoragefile.h | 3 ++- tools/virsh-domain.c | 6 ++ tools/virsh.pod | 6 -- 7 files changed, 61 insertions(+), 8 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/n] dumpxml: add flag to virDomainGetXMLDesc
The information in virDomainGetBlockInfo() is important for clients that use qcow2 format on LVM block devices - by tracking the allocation in relation to the physical size, management can tell if a disk needs to be expanded before the guest (file system contents) and/or qemu (copy-on-write differing more from a backing file) runs out of space. Normally, only the active layer matters, but during a block commit operation, the allocation of the backing file ALSO grows, and management would like to track that growth. Right now, virDomainGetBlockInfo() can only convey information about the active layer of a disk, via a single API call per disk. It can also be easily extended to support vda[1] notation that we recently added for blockcommit and friends, to get similar information about a backing element; but that still implies one call per layer, which adds up to a lot of overhead. This API addition will make it possible to grab this information for ALL guest disks in a single call, by letting the user request additional information about each disk in the backing chain to be output as part of the domain XML. My ultimate goal is to have this flag and virStorageVolGetXMLDesc() expose the same bits of information about a given storage volume (there are slight incompatiblities in the XML naming that we'll have to keep for back-compat sake, but the idea is to get all the information out there). * include/libvirt/libvirt.h.in (virDomainXMLFlags): Add new flag. * src/libvirt.c (virDomainGetXMLDesc): Document it. (virDomainSaveImageGetXMLDesc, virDomainSnapshotGetXMLDesc): For now, mention the flag won't help here. * tools/virsh-domain.c (cmdDumpXML): Add new flag. * tools/virsh.pod (dumpxml): Document it. Signed-off-by: Eric Blake ebl...@redhat.com --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c| 15 +++ tools/virsh-domain.c | 6 ++ tools/virsh.pod | 6 -- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c2f9d26..40f4e0c 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2082,6 +2082,7 @@ typedef enum { VIR_DOMAIN_XML_INACTIVE = (1 1), /* dump inactive domain information */ VIR_DOMAIN_XML_UPDATE_CPU = (1 2), /* update guest CPU requirements according to host CPU */ VIR_DOMAIN_XML_MIGRATABLE = (1 3), /* dump XML suitable for migration */ +VIR_DOMAIN_XML_BLOCK_INFO = (1 4), /* include storage volume information about each disk source */ } virDomainXMLFlags; char * virDomainGetXMLDesc (virDomainPtr domain, diff --git a/src/libvirt.c b/src/libvirt.c index f7e5a37..1020cc5 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2806,8 +2806,9 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml, * * No security-sensitive data will be included unless @flags contains * VIR_DOMAIN_XML_SECURE; this flag is rejected on read-only - * connections. For this API, @flags should not contain either - * VIR_DOMAIN_XML_INACTIVE or VIR_DOMAIN_XML_UPDATE_CPU. + * connections. For this API, @flags should not contain + * VIR_DOMAIN_XML_INACTIVE, VIR_DOMAIN_XML_UPDATE_CPU, or + * VIR_DOMAIN_XML_BLOCK_INFO. * * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of * error. The caller must free() the returned value. @@ -4354,6 +4355,11 @@ virDomainGetControlInfo(virDomainPtr domain, * describing CPU capabilities is modified to match actual * capabilities of the host. * + * If @flags contains VIR_DOMAIN_XML_BLOCK_INFO, the listing for each + * disk device will contain additional information such as capacity + * and allocation, similar to what is displayed by virStorageVolGetXMLDesc(), + * and avoiding the need to call virDomainGetBlockInfo() separately. + * * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error. * the caller must free() the returned value. */ @@ -18507,8 +18513,9 @@ virDomainSnapshotCreateXML(virDomainPtr domain, * * No security-sensitive data will be included unless @flags contains * VIR_DOMAIN_XML_SECURE; this flag is rejected on read-only - * connections. For this API, @flags should not contain either - * VIR_DOMAIN_XML_INACTIVE or VIR_DOMAIN_XML_UPDATE_CPU. + * connections. For this API, @flags should not contain + * VIR_DOMAIN_XML_INACTIVE, VIR_DOMAIN_XML_UPDATE_CPU, or + * VIR_DOMAIN_XML_BLOCK_INFO. * * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error. * the caller must free() the returned value. diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 435d045..8f97c48 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8838,6 +8838,10 @@ static const vshCmdOptDef opts_dumpxml[] = { .type = VSH_OT_BOOL, .help = N_(provide XML suitable for migrations) }, +{.name = block-info, + .type =
[libvirt] [PATCH 2/6] qemu: Fix old tcp:host URIs more cleanly
For compatibility with old libvirt we need to support both tcp:host and tcp://host migration URIs. Let's make the code that parses them a bit cleaner. Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Version 3: - new patch to make qemu: RDMA migration support a bit more straightforward src/qemu/qemu_migration.c | 77 --- 1 file changed, 33 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d738f9b..5741de2 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2817,6 +2817,29 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver, } +static virURIPtr +qemuMigrationParseURI(const char *uri, bool *wellFormed) +{ +char *tmp = NULL; +virURIPtr parsed; + +/* For compatibility reasons tcp://... URIs are sent as tcp:... + * We need to transform them to a well-formed URI before parsing. */ +if (STRPREFIX(uri, tcp:) !STRPREFIX(uri + 4, //)) { +if (virAsprintf(tmp, tcp://%s, uri + 4) 0) +return NULL; +uri = tmp; +} + +parsed = virURIParse(uri); +if (parsed wellFormed) +*wellFormed = !tmp; +VIR_FREE(tmp); + +return parsed; +} + + int qemuMigrationPrepareDirect(virQEMUDriverPtr driver, virConnectPtr dconn, @@ -2834,11 +2857,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, unsigned short port = 0; bool autoPort = true; char *hostname = NULL; -const char *p; -char *uri_str = NULL; int ret = -1; virURIPtr uri = NULL; -bool well_formed_uri = true; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *migrateHost = cfg-migrateHost; @@ -2890,34 +2910,18 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, * compatibility with old targets. We at least make the * new targets accept both syntaxes though. */ -/* Caller frees */ if (virAsprintf(uri_out, tcp:%s:%d, hostname, port) 0) goto cleanup; } else { -/* Check the URI starts with tcp:. We will escape the - * URI when passing it to the qemu monitor, so bad - * characters in hostname part don't matter. - */ -if (!(p = STRSKIP(uri_in, tcp:))) { -virReportError(VIR_ERR_INVALID_ARG, %s, - _(only tcp URIs are supported for KVM/QEMU - migrations)); +bool well_formed_uri; + +if (!(uri = qemuMigrationParseURI(uri_in, well_formed_uri))) goto cleanup; -} -/* Convert uri_in to well-formed URI with // after tcp: */ -if (!(STRPREFIX(uri_in, tcp://))) { -well_formed_uri = false; -if (virAsprintf(uri_str, tcp://%s, p) 0) -goto cleanup; -} - -uri = virURIParse(uri_str ? uri_str : uri_in); -VIR_FREE(uri_str); - -if (uri == NULL) { -virReportError(VIR_ERR_INVALID_ARG, _(unable to parse URI: %s), - uri_in); +if (STRNEQ(uri-scheme, tcp)) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _(unsupported scheme %s in migration URI %s), + uri-scheme, uri_in); goto cleanup; } @@ -2931,27 +2935,22 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, if (virPortAllocatorAcquire(driver-migrationPorts, port) 0) goto cleanup; +/* Send well-formed URI only if uri_in was well-formed */ if (well_formed_uri) { uri-port = port; - -/* Caller frees */ if (!(*uri_out = virURIFormat(uri))) goto cleanup; } else { -/* Caller frees */ if (virAsprintf(uri_out, %s:%d, uri_in, port) 0) goto cleanup; } - } else { port = uri-port; autoPort = false; } } -if (*uri_out) -VIR_DEBUG(Generated uri_out=%s, *uri_out); - +VIR_DEBUG(Generated uri_out=%s, *uri_out); ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, def, origname, NULL, port, autoPort, listenAddress, flags); @@ -3704,17 +3703,7 @@ static int doNativeMigrate(virQEMUDriverPtr driver, cookieout, cookieoutlen, flags, resource, NULLSTR(graphicsuri)); -if (STRPREFIX(uri, tcp:) !STRPREFIX(uri, tcp://)) { -char *tmp; -/* HACK: source host generates bogus URIs, so fix them up */ -if (virAsprintf(tmp, tcp://%s, uri + strlen(tcp:)) 0) -return -1; -uribits = virURIParse(tmp); -VIR_FREE(tmp); -} else { -uribits = virURIParse(uri);
[libvirt] [PATCH 0/6] RDMA migration support
This is a modified version of RDMA migration patches sent back in January by Michael R. Hines. See individual patches for (numerous) changes since v2. Jiri Denemark (3): qemu: Fix old tcp:host URIs more cleanly qemu: Prepare support for arbitrary migration protocol qemu: Add RDMA migration capabilities Michael R. Hines (3): qemu: Expose additional migration statistics qemu: RDMA migration support qemu: Memory pre-pinning support for RDMA migration include/libvirt/libvirt.h.in | 26 + src/qemu/qemu.conf |8 + src/qemu/qemu_capabilities.c | 32 +- src/qemu/qemu_capabilities.h |1 + src/qemu/qemu_command.c |8 + src/qemu/qemu_domain.c | 18 + src/qemu/qemu_migration.c| 216 +- src/qemu/qemu_migration.h|3 +- src/qemu/qemu_monitor.c | 25 +- src/qemu/qemu_monitor.h | 13 + src/qemu/qemu_monitor_json.c | 61 +- src/qemu/qemu_monitor_json.h |2 + tests/qemucapabilitiesdata/caps_1.2.2-1.replies | 10 + tests/qemucapabilitiesdata/caps_1.3.1-1.replies | 10 + tests/qemucapabilitiesdata/caps_1.4.2-1.replies | 10 + tests/qemucapabilitiesdata/caps_1.5.3-1.replies | 10 + tests/qemucapabilitiesdata/caps_1.6.0-1.replies | 22 + tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 22 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 162 ++ tests/qemucapabilitiesdata/caps_2.1.1-1.replies | 3264 ++ tests/qemucapabilitiestest.c |1 + tools/virsh-domain.c | 34 + 22 files changed, 3886 insertions(+), 72 deletions(-) create mode 100644 tests/qemucapabilitiesdata/caps_2.1.1-1.caps create mode 100644 tests/qemucapabilitiesdata/caps_2.1.1-1.replies -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] qemu: Memory pre-pinning support for RDMA migration
From: Michael R. Hines mrhi...@us.ibm.com RDMA Live migration requires registering memory with the hardware, and thus QEMU offers a new 'capability' to pre-register / mlock() the guest memory in advance for higher RDMA performance before the migration begins. This capability is disabled by default, which means QEMU will register the memory with the hardware in an on-demand basis. This patch exposes this capability with the following example usage: virsh migrate --live --rdma-pin-all --migrateuri rdma://hostname domain qemu+ssh://hostname/system Signed-off-by: Michael R. Hines mrhi...@us.ibm.com Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Version 3: - moved rdma-pin-all capability into qemu: Add RDMA migration capabilities patch - removed magic computation of mlock memory limit include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_migration.c| 49 src/qemu/qemu_migration.h| 3 ++- tools/virsh-domain.c | 7 +++ 4 files changed, 59 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 702f797..a028e2d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1224,6 +1224,7 @@ typedef enum { VIR_MIGRATE_COMPRESSED= (1 11), /* compress data during migration */ VIR_MIGRATE_ABORT_ON_ERROR= (1 12), /* abort migration on I/O errors happened during migration */ VIR_MIGRATE_AUTO_CONVERGE = (1 13), /* force convergence */ +VIR_MIGRATE_RDMA_PIN_ALL = (1 14), /* RDMA memory pinning */ } virDomainMigrateFlags; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b59e94d..2daac48 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1874,6 +1874,46 @@ qemuMigrationSetAutoConverge(virQEMUDriverPtr driver, static int +qemuMigrationSetPinAll(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob job) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +int ret; + +if (qemuDomainObjEnterMonitorAsync(driver, vm, job) 0) +return -1; + +ret = qemuMonitorGetMigrationCapability( +priv-mon, +QEMU_MONITOR_MIGRATION_CAPS_RDMA_PIN_ALL); + +if (ret 0) { +goto cleanup; +} else if (ret == 0) { +if (job == QEMU_ASYNC_JOB_MIGRATION_IN) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(rdma pinning migration is not supported by + target QEMU binary)); +} else { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(rdma pinning migration is not supported by + source QEMU binary)); +} +ret = -1; +goto cleanup; +} + +ret = qemuMonitorSetMigrationCapability( +priv-mon, +QEMU_MONITOR_MIGRATION_CAPS_RDMA_PIN_ALL); + + cleanup: +qemuDomainObjExitMonitor(driver, vm); +return ret; +} + +static int qemuMigrationWaitForSpice(virQEMUDriverPtr driver, virDomainObjPtr vm) { @@ -2709,6 +2749,10 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, goto stop; } +if (flags VIR_MIGRATE_RDMA_PIN_ALL +qemuMigrationSetPinAll(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN) 0) +goto stop; + if (mig-lockState) { VIR_DEBUG(Received lockstate %s, mig-lockState); VIR_FREE(priv-lockState); @@ -3530,6 +3574,11 @@ qemuMigrationRun(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_OUT) 0) goto cleanup; +if (flags VIR_MIGRATE_RDMA_PIN_ALL +qemuMigrationSetPinAll(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT) 0) +goto cleanup; + if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT) 0) goto cleanup; diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 3fa68dc..e7a90c3 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -40,7 +40,8 @@ VIR_MIGRATE_OFFLINE | \ VIR_MIGRATE_COMPRESSED | \ VIR_MIGRATE_ABORT_ON_ERROR | \ - VIR_MIGRATE_AUTO_CONVERGE) + VIR_MIGRATE_AUTO_CONVERGE |\ + VIR_MIGRATE_RDMA_PIN_ALL) /* All supported migration parameters and their types. */ # define QEMU_MIGRATION_PARAMETERS \ diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 105b99e..a6ced5f 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9212,6 +9212,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_BOOL, .help = N_(force convergence during live migration) }, +{.name = rdma-pin-all, +
[libvirt] [PATCH 3/6] qemu: Prepare support for arbitrary migration protocol
Currently we only support TCP protocol for native QEMU migration but this is going to be changed. Let's make the code more general and remove hardcoded TCP protocol from several places. Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Version 3: - separated from qemu: RDMA migration support src/qemu/qemu_migration.c | 36 src/qemu/qemu_monitor.c | 3 ++- src/qemu/qemu_monitor.h | 1 + 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5741de2..d0e2653 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2457,6 +2457,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, virDomainDefPtr *def, const char *origname, virStreamPtr st, +const char *protocol, unsigned short port, bool autoPort, const char *listenAddress, @@ -2569,6 +2570,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, struct addrinfo *info = NULL; struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG, .ai_socktype = SOCK_STREAM }; +const char *incFormat; if (getaddrinfo(::, NULL, hints, info) == 0) { freeaddrinfo(info); @@ -2605,21 +2607,27 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, } else { /* listenAddress is a hostname */ } -} else { +} else if (qemuIPv6Capable hostIPv6Capable) { /* Listen on :: instead of 0.0.0.0 if QEMU understands it * and there is at least one IPv6 address configured */ -listenAddress = qemuIPv6Capable hostIPv6Capable ? -encloseAddress = true, :: : 0.0.0.0; +listenAddress = ::; +encloseAddress = true; +} else { +listenAddress = 0.0.0.0; } -/* QEMU will be started with -incoming [IPv6 addr]:port, - * -incoming IPv4 addr:port or -incoming hostname:port +/* QEMU will be started with + * -incoming protocol:[IPv6 addr]:port, + * -incoming protocol:IPv4 addr:port, or + * -incoming protocol:hostname:port */ -if ((encloseAddress - virAsprintf(migrateFrom, tcp:[%s]:%d, listenAddress, port) 0) || -(!encloseAddress - virAsprintf(migrateFrom, tcp:%s:%d, listenAddress, port) 0)) +if (encloseAddress) +incFormat = %s:[%s]:%d; +else +incFormat = %s:%s:%d; +if (virAsprintf(migrateFrom, incFormat, +protocol, listenAddress, port) 0) goto cleanup; } @@ -2812,7 +2820,7 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver, ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, def, origname, - st, 0, false, NULL, flags); + st, NULL, 0, false, NULL, flags); return ret; } @@ -2953,7 +2961,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, VIR_DEBUG(Generated uri_out=%s, *uri_out); ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, def, origname, - NULL, port, autoPort, listenAddress, flags); + NULL, uri ? uri-scheme : tcp, + port, autoPort, listenAddress, flags); cleanup: virURIFree(uri); VIR_FREE(hostname); @@ -3169,6 +3178,7 @@ struct _qemuMigrationSpec { enum qemuMigrationDestinationType destType; union { struct { +const char *protocol; const char *name; int port; } host; @@ -3536,6 +3546,7 @@ qemuMigrationRun(virQEMUDriverPtr driver, switch (spec-destType) { case MIGRATION_DEST_HOST: ret = qemuMonitorMigrateToHost(priv-mon, migrate_flags, + spec-dest.host.protocol, spec-dest.host.name, spec-dest.host.port); break; @@ -3676,7 +3687,7 @@ qemuMigrationRun(virQEMUDriverPtr driver, goto cleanup; } -/* Perform migration using QEMU's native TCP migrate support, +/* Perform migration using QEMU's native migrate support, * not encrypted obviously */ static int doNativeMigrate(virQEMUDriverPtr driver, @@ -3710,6 +3721,7 @@ static int doNativeMigrate(virQEMUDriverPtr driver, spec.destType = MIGRATION_DEST_CONNECT_HOST; else spec.destType = MIGRATION_DEST_HOST; +spec.dest.host.protocol = uribits-scheme; spec.dest.host.name = uribits-server;
[libvirt] [PATCH 5/6] qemu: RDMA migration support
From: Michael R. Hines mrhi...@us.ibm.com This patch adds support for RDMA protocol in migration URIs. USAGE: $ virsh migrate --live --migrateuri rdma://hostname domain qemu+ssh://hostname/system Since libvirt runs QEMU in a pretty restricted environment, several files needs to be added to cgroup_device_acl (in qemu.conf) for QEMU to be able to access the host's infiniband hardware. Full documenation of the feature can be found on QEMU wiki: http://wiki.qemu.org/Features/RDMALiveMigration Signed-off-by: Michael R. Hines mrhi...@us.ibm.com Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: The question is whether the IB devices should be added to cgroup_device_acl by default or not... Version 3: - moved capabilities code into a separate patch - got rid of migration URI hacks - removed hacks that disabled IPv6 with RDMA - moved refactoring into a dedicated patch - documented IB devices which need to be added to cgroup acl in qemu.conf - forbid RDMA migrations unless memory hard limit is set until we have a better plan for setting limits for mlock - set QEMU's RLIMIT_MEMLOCK to memory hard_limit before starting RDMA migration - check if RDMA migration is supported by source QEMU before trying to migrate src/qemu/qemu.conf| 8 src/qemu/qemu_command.c | 8 src/qemu/qemu_migration.c | 39 +-- 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 79bba36..92ca715 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -274,6 +274,14 @@ #/dev/ptmx, /dev/kvm, /dev/kqemu, #/dev/rtc,/dev/hpet, /dev/vfio/vfio #] +# +# RDMA migration requires the following extra files to be added to the list: +# /dev/infiniband/rdma_cm, +# /dev/infiniband/issm0, +# /dev/infiniband/issm1, +# /dev/infiniband/umad0, +# /dev/infiniband/umad1, +# /dev/infiniband/uverbs0 # The default format for Qemu/KVM guest save images is raw; that is, the diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a892d99..fceed62 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9399,6 +9399,14 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } virCommandAddArg(cmd, migrateFrom); +} else if (STRPREFIX(migrateFrom, rdma)) { +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_RDMA)) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, + _(incoming RDMA migration is not supported + with this QEMU binary)); +goto error; +} +virCommandAddArg(cmd, migrateFrom); } else if (STREQ(migrateFrom, stdio)) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) { virCommandAddArgFormat(cmd, fd:%d, migrateFd); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d0e2653..b59e94d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -56,6 +56,7 @@ #include virhook.h #include virstring.h #include virtypedparam.h +#include virprocess.h #define VIR_FROM_THIS VIR_FROM_QEMU @@ -2653,6 +2654,13 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, QEMU_MIGRATION_COOKIE_NBD))) goto cleanup; +if (STREQ(protocol, rdma) !vm-def-mem.hard_limit) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(cannot start RDMA migration with no memory hard + limit set)); +goto cleanup; +} + if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN) 0) goto cleanup; qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PREPARE); @@ -2696,6 +2704,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_IN) 0) goto stop; +if (STREQ(protocol, rdma) +virProcessSetMaxMemLock(vm-pid, vm-def-mem.hard_limit 10) 0) { +goto stop; +} + if (mig-lockState) { VIR_DEBUG(Received lockstate %s, mig-lockState); VIR_FREE(priv-lockState); @@ -2926,7 +2939,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, if (!(uri = qemuMigrationParseURI(uri_in, well_formed_uri))) goto cleanup; -if (STRNEQ(uri-scheme, tcp)) { +if (STRNEQ(uri-scheme, tcp) +STRNEQ(uri-scheme, rdma)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, _(unsupported scheme %s in migration URI %s), uri-scheme, uri_in); @@ -3545,6 +3559,11 @@ qemuMigrationRun(virQEMUDriverPtr driver, switch (spec-destType) { case MIGRATION_DEST_HOST: +if (STREQ(spec-dest.host.protocol, rdma) +
[libvirt] [PATCH 4/6] qemu: Add RDMA migration capabilities
Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Version 3: - separated from qemu: RDMA migration support - switched from version based to feature based probing - fixed existing capabilities tests - added new capabilities test for QEMU 2.1.1 which supports rdma-pin-all migration capability src/qemu/qemu_capabilities.c | 32 +- src/qemu/qemu_capabilities.h |1 + src/qemu/qemu_monitor.c | 22 +- src/qemu/qemu_monitor.h |3 + src/qemu/qemu_monitor_json.c | 44 +- src/qemu/qemu_monitor_json.h |2 + tests/qemucapabilitiesdata/caps_1.2.2-1.replies | 10 + tests/qemucapabilitiesdata/caps_1.3.1-1.replies | 10 + tests/qemucapabilitiesdata/caps_1.4.2-1.replies | 10 + tests/qemucapabilitiesdata/caps_1.5.3-1.replies | 10 + tests/qemucapabilitiesdata/caps_1.6.0-1.replies | 22 + tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 22 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 162 ++ tests/qemucapabilitiesdata/caps_2.1.1-1.replies | 3264 ++ tests/qemucapabilitiestest.c |1 + 15 files changed, 3602 insertions(+), 13 deletions(-) create mode 100644 tests/qemucapabilitiesdata/caps_2.1.1-1.caps create mode 100644 tests/qemucapabilitiesdata/caps_2.1.1-1.replies diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9f8868d..30b3c5d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -269,6 +269,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, splash-timeout, /* 175 */ iothread, + migrate-rdma, ); @@ -993,9 +994,9 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache) if (virQEMUCapsInitPages(caps) 0) VIR_WARN(Failed to get pages info); -/* Add domain migration transport URI */ -virCapabilitiesAddHostMigrateTransport(caps, - tcp); +/* Add domain migration transport URIs */ +virCapabilitiesAddHostMigrateTransport(caps, tcp); +virCapabilitiesAddHostMigrateTransport(caps, rdma); /* QEMU can support pretty much every arch that exists, * so just probe for them all - we gracefully fail @@ -1435,6 +1436,10 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { rtc-reset-reinjection, QEMU_CAPS_RTC_RESET_REINJECTION }, }; +struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { +{ rdma-pin-all, QEMU_CAPS_MIGRATE_RDMA }, +}; + struct virQEMUCapsStringFlags virQEMUCapsEvents[] = { { BALLOON_CHANGE, QEMU_CAPS_BALLOON_EVENT }, { SPICE_MIGRATE_COMPLETED, QEMU_CAPS_SEAMLESS_MIGRATION }, @@ -2476,6 +2481,25 @@ virQEMUCapsProbeQMPCommandLine(virQEMUCapsPtr qemuCaps, return 0; } +static int +virQEMUCapsProbeQMPMigrationCapabilities(virQEMUCapsPtr qemuCaps, + qemuMonitorPtr mon) +{ +char **caps = NULL; +int ncaps; + +if ((ncaps = qemuMonitorGetMigrationCapabilities(mon, caps)) 0) +return -1; + +virQEMUCapsProcessStringFlags(qemuCaps, + ARRAY_CARDINALITY(virQEMUCapsMigration), + virQEMUCapsMigration, + ncaps, caps); +virQEMUCapsFreeStringList(ncaps, caps); + +return 0; +} + int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon) { @@ -3168,6 +3192,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, goto cleanup; if (virQEMUCapsProbeQMPCommandLine(qemuCaps, mon) 0) goto cleanup; +if (virQEMUCapsProbeQMPMigrationCapabilities(qemuCaps, mon) 0) +goto cleanup; ret = 0; cleanup: diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0980c00..6a8b84f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -216,6 +216,7 @@ typedef enum { QEMU_CAPS_RTC_RESET_REINJECTION = 174, /* rtc-reset-reinjection monitor command */ QEMU_CAPS_SPLASH_TIMEOUT = 175, /* -boot splash-time */ QEMU_CAPS_OBJECT_IOTHREAD= 176, /* -object iothread */ +QEMU_CAPS_MIGRATE_RDMA = 177, /* have rdma migration */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 31ab37d..0f48398 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -121,7 +121,7 @@ VIR_ENUM_IMPL(qemuMonitorMigrationStatus, VIR_ENUM_IMPL(qemuMonitorMigrationCaps, QEMU_MONITOR_MIGRATION_CAPS_LAST, - xbzrle, auto-converge) + xbzrle, auto-converge, rdma-pin-all) VIR_ENUM_IMPL(qemuMonitorVMStatus, QEMU_MONITOR_VM_STATUS_LAST, @@ -3762,6 +3762,26 @@ char
[libvirt] [PATCH 1/6] qemu: Expose additional migration statistics
From: Michael R. Hines mrhi...@us.ibm.com RDMA migration uses the 'setup' state in QEMU to optionally lock all memory before the migration starts. The total time spent in this state is exposed as VIR_DOMAIN_JOB_SETUP_TIME. Additionally, QEMU also exports migration throughput (mbps) for both memory and disk, so let's add them too: VIR_DOMAIN_JOB_MEMORY_BPS, VIR_DOMAIN_JOB_DISK_BPS. Signed-off-by: Michael R. Hines mrhi...@us.ibm.com Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Version 3: - changed MBPS to BPS - added support for both memory and disk BPS - changed BPS to ULLONG - added code to transfer the new statistics to the destination daemon when migration completes - added the new parameters to virsh include/libvirt/libvirt.h.in | 25 + src/qemu/qemu_domain.c | 18 ++ src/qemu/qemu_migration.c| 17 + src/qemu/qemu_monitor.h | 9 + src/qemu/qemu_monitor_json.c | 17 + tools/virsh-domain.c | 27 +++ 6 files changed, 113 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c2f9d26..702f797 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4388,6 +4388,15 @@ int virDomainAbortJob(virDomainPtr dom); #define VIR_DOMAIN_JOB_DOWNTIME downtime /** + * VIR_DOMAIN_JOB_SETUP_TIME: + * + * virDomainGetJobStats field: total time in milliseconds spent preparing + * the migration in the 'setup' phase before the iterations begin, as + * VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_JOB_SETUP_TIME setup_time + +/** * VIR_DOMAIN_JOB_DATA_TOTAL: * * virDomainGetJobStats field: total number of bytes supposed to be @@ -4485,6 +4494,14 @@ int virDomainAbortJob(virDomainPtr dom); #define VIR_DOMAIN_JOB_MEMORY_NORMAL_BYTES memory_normal_bytes /** + * VIR_DOMAIN_JOB_MEMORY_BPS: + * + * virDomainGetJobStats field: network throughput used while migrating + * memory in Bytes per second, as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_JOB_MEMORY_BPS memory_bps + +/** * VIR_DOMAIN_JOB_DISK_TOTAL: * * virDomainGetJobStats field: as VIR_DOMAIN_JOB_DATA_TOTAL but only @@ -4515,6 +4532,14 @@ int virDomainAbortJob(virDomainPtr dom); #define VIR_DOMAIN_JOB_DISK_REMAINING disk_remaining /** + * VIR_DOMAIN_JOB_DISK_BPS: + * + * virDomainGetJobStats field: network throughput used while migrating + * disks in Bytes per second, as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_JOB_DISK_BPS disk_bps + +/** * VIR_DOMAIN_JOB_COMPRESSION_CACHE: * * virDomainGetJobStats field: size of the cache (in bytes) used for diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 863ab09..9b3edd7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -304,6 +304,12 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, status-downtime) 0) goto error; +if (status-setup_time_set +virTypedParamsAddULLong(par, npar, maxpar, +VIR_DOMAIN_JOB_SETUP_TIME, +status-setup_time) 0) +goto error; + if (virTypedParamsAddULLong(par, npar, maxpar, VIR_DOMAIN_JOB_DATA_TOTAL, status-ram_total + @@ -329,6 +335,12 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, status-ram_remaining) 0) goto error; +if (status-ram_bps +virTypedParamsAddULLong(par, npar, maxpar, +VIR_DOMAIN_JOB_MEMORY_BPS, +status-ram_bps) 0) +goto error; + if (status-ram_duplicate_set) { if (virTypedParamsAddULLong(par, npar, maxpar, VIR_DOMAIN_JOB_MEMORY_CONSTANT, @@ -353,6 +365,12 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, status-disk_remaining) 0) goto error; +if (status-disk_bps +virTypedParamsAddULLong(par, npar, maxpar, +VIR_DOMAIN_JOB_DISK_BPS, +status-disk_bps) 0) +goto error; + if (status-xbzrle_set) { if (virTypedParamsAddULLong(par, npar, maxpar, VIR_DOMAIN_JOB_COMPRESSION_CACHE, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index ce1a5cd..d738f9b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -636,6 +636,10 @@ qemuMigrationCookieStatisticsXMLFormat(virBufferPtr buf, virBufferAsprintf(buf, %1$s%2$llu/%1$s\n, VIR_DOMAIN_JOB_DOWNTIME, status-downtime); +if (status-setup_time_set) +virBufferAsprintf(buf,
Re: [libvirt] [PATCH 0/6] RDMA migration support
On Wed, Sep 17, 2014 at 16:53:02 +0200, Jiri Denemark wrote: This is a modified version of RDMA migration patches sent back in January by Michael R. Hines. See individual patches for (numerous) changes since v2. Jiri Denemark (3): qemu: Fix old tcp:host URIs more cleanly qemu: Prepare support for arbitrary migration protocol qemu: Add RDMA migration capabilities Michael R. Hines (3): qemu: Expose additional migration statistics qemu: RDMA migration support qemu: Memory pre-pinning support for RDMA migration And this whole series should have been obviously marked as v3... Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On Mon, Sep 15, 2014 at 04:30:46PM -0600, Eric Blake wrote: On 09/11/2014 05:43 AM, Ján Tomko wrote: Add the following attributes: csum, gso, guest_tso4, guest_tso6, guest_ecn to the driver element of network interface which control the virtio-net device properties of the same names. --- docs/formatdomain.html.in | 27 docs/schemas/domaincommon.rng | 25 +++ src/conf/domain_conf.c | 81 ++ src/conf/domain_conf.h | 5 ++ .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 + tests/qemuxml2xmltest.c| 1 + 6 files changed, 171 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a2ea758..5b2758a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null lt;model type='virtio'/gt; blt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5'/gt;/b lt;/interfacegt; +lt;interface type='network'gt; + lt;source network='default'/gt; + lt;target dev='vnet2'/gt; + lt;model type='virtio'/gt; + blt;driver csum='off' gso='off' guest_tso4='off' guest_tso6='off' guest_ecn='off'/gt;/b +lt;/interfacegt; Are we stuck with names with underscores in our XML? I'm still not sure if we've come up with the best naming for exposing all these knobs. I'm not really convinced having a 'guest_' prefix really buys us anything here, since there's no naming clash to avoid. Why don't we just kill the 'guest_' prefixes. NB, remember that precisely matching QEMU naming is a non-goal, we should be designing something that makes sense in general. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] rpc: make daemon spawning a bit more intelligent
On Wed, Sep 17, 2014 at 10:59:21AM -0400, John Ferlan wrote: On 09/17/2014 10:00 AM, Martin Kletzander wrote: On Tue, Sep 16, 2014 at 10:22:53AM -0400, John Ferlan wrote: On 09/16/2014 05:16 AM, Martin Kletzander wrote: This way it behaves more like the daemon itself does (acquiring a pidfile, deleting the socket before binding, etc.). Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138604 Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/rpc/virnetsocket.c | 65 +++--- 1 file changed, 57 insertions(+), 8 deletions(-) The error path/retry logic needs a tweak... I added some inline thinking since we don't have a virtual whiteboard to share on this! Yes, it does. I didn't think it through from scratch, just adjusted to your comments. This time I went through it few times. Just let me confirm I understood what you meant everywhere. You did :-) ...snip... Wow, I just reached this part of the mail when I wrote it already :) Funny how that happens. My current diff to the previous version looks like this: diff --git i/src/rpc/virnetsocket.c w/src/rpc/virnetsocket.c index 7be1492..e0efb14 100644 --- i/src/rpc/virnetsocket.c +++ w/src/rpc/virnetsocket.c @@ -596,7 +596,6 @@ int virNetSocketNewConnectUNIX(const char *path, goto error; pidfd = virPidFileAcquirePath(pidpath, false, getpid()); -VIR_FREE(pidpath); if (pidfd 0) { /* * This can happen in a very rare case of two clients spawning two @@ -650,6 +649,7 @@ int virNetSocketNewConnectUNIX(const char *path, * time without spawning the daemon. */ spawnDaemon = false; +virPidFileDeletePath(pidpath); VIR_FORCE_CLOSE(pidfd); VIR_FORCE_CLOSE(passfd); goto retry; @@ -674,6 +674,7 @@ int virNetSocketNewConnectUNIX(const char *path, * virCommandHook inside a virNetSocketForkDaemon(). */ VIR_FORCE_CLOSE(pidfd); +pidfd = -1; VIR_FORCE_CLOSE() will do this for you if (virNetSocketForkDaemon(binary, passfd) 0) goto error; } @@ -687,10 +688,12 @@ int virNetSocketNewConnectUNIX(const char *path, if (!(*retsock = virNetSocketNew(localAddr, remoteAddr, true, fd, -1, 0))) goto error; +VIR_FREE(pidpath); + return 0; error: -if (pidfd 0) +if (pidfd = 0) virPidFileDeletePath(pidpath); VIR_FREE(pidpath); VIR_FORCE_CLOSE(fd); -- Is that fine or should I use that goto error; everywhere after acquiring the pidfile or is it better for you to see it in another version? This is fine - I think things are now covered. ACK Thanks for you thorough review, I squashed it in, remove that one unnecessary pidfd = -1 and pushed it. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/2] Expose UEFI binary path
On 09/17/2014 06:15 AM, Michal Privoznik wrote: This is practically reworked v1 from Cole. Cole Robinson (1): domaincaps: Expose UEFI binary path, if it exists Michal Privoznik (1): qemu_capabilities: Change virQEMUCapsFillDomainCaps signature docs/formatdomaincaps.html.in | 6 ++ docs/schemas/domaincaps.rng| 17 -- src/conf/domain_capabilities.c | 29 ++ src/conf/domain_capabilities.h | 8 +++ src/qemu/qemu_capabilities.c | 53 + src/qemu/qemu_capabilities.h | 9 ++- src/qemu/qemu_driver.c | 7 ++- tests/domaincapsschemadata/domaincaps-full.xml | 2 + .../domaincaps-qemu_1.6.50-1.xml | 1 + tests/domaincapstest.c | 66 ++ 10 files changed, 167 insertions(+), 31 deletions(-) I'm now seeing test failures: FAIL: domaincapstest TEST: domaincapstest 1) basic ... OK 2) full ... OK 3) qemu_1.6.50-1 ... Offset 196 Expect [value/usr/share/OVMF/OVMF_CODE.fd/value e] Actual [e] ... FAILED -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Refactor save image opening and add restore hook
This series refactors (splits) the save image open function into separate chunks and introduces a filter-hook that is called when restoring a save image. Peter Krempa (5): qemu: save image: Split out user provided XML checker qemu: save image: Add possibility to return XML stored in the image qemu: save image: Split out new definition check/update qemu: save image: Split out checks done only when editing the save img qemu: hook: Provide hook when restoring a domain save image docs/hooks.html.in | 11 +++ src/qemu/qemu_driver.c | 261 ++--- src/util/virhook.c | 3 +- src/util/virhook.h | 1 + 4 files changed, 195 insertions(+), 81 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] qemu: save image: Split out new definition check/update
Split out the call to the update method only to places where it is actually used rather than having a mega-method that does all the stuff. --- src/qemu/qemu_driver.c | 26 +++--- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a276ea5..0151fd2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5495,16 +5495,6 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, VIR_DOMAIN_XML_INACTIVE))) goto error; -if (xmlin) { -virDomainDefPtr tmp; - -if (!(tmp = qemuDomainSaveImageUpdateDef(driver, def, xmlin))) -goto error; - -virDomainDefFree(def); -def = tmp; -} - if (xmlout) *xmlout = xml; else @@ -5647,6 +5637,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, { virQEMUDriverPtr driver = conn-privateData; virDomainDefPtr def = NULL; +virDomainDefPtr newdef = NULL; virDomainObjPtr vm = NULL; int fd = -1; int ret = -1; @@ -5673,6 +5664,14 @@ qemuDomainRestoreFlags(virConnectPtr conn, if (virDomainRestoreFlagsEnsureACL(conn, def) 0) goto cleanup; +if (dxml) { +if (!(newdef = qemuDomainSaveImageUpdateDef(driver, def, dxml))) +goto cleanup; + +virDomainDefFree(def); +def = newdef; +} + if (!(vm = virDomainObjListAdd(driver-domains, def, driver-xmlopt, VIR_DOMAIN_OBJ_LIST_ADD_LIVE | @@ -5749,6 +5748,7 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, virQEMUDriverPtr driver = conn-privateData; int ret = -1; virDomainDefPtr def = NULL; +virDomainDefPtr newdef = NULL; int fd = -1; virQEMUSaveHeader header; char *xml = NULL; @@ -5776,7 +5776,10 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, if (virDomainSaveImageDefineXMLEnsureACL(conn, def) 0) goto cleanup; -xml = qemuDomainDefFormatXML(driver, def, +if (!(newdef = qemuDomainSaveImageUpdateDef(driver, def, dxml))) +goto cleanup; + +xml = qemuDomainDefFormatXML(driver, newdef, VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_MIGRATABLE); @@ -5807,6 +5810,7 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, cleanup: virDomainDefFree(def); +virDomainDefFree(newdef); VIR_FORCE_CLOSE(fd); VIR_FREE(xml); return ret; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] qemu: save image: Add possibility to return XML stored in the image
Add a new parameter that will allow to return the XML stored in the save image for further manipulation and adjust the callers. This option will be used in later patches. --- src/qemu/qemu_driver.c | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e41a08e..a276ea5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5382,6 +5382,7 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, const char *path, virDomainDefPtr *ret_def, virQEMUSaveHeaderPtr ret_header, +char **xmlout, bool bypass_cache, virFileWrapperFdPtr *wrapperFd, const char *xmlin, int state, bool edit, @@ -5504,7 +5505,10 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, def = tmp; } -VIR_FREE(xml); +if (xmlout) +*xmlout = xml; +else +VIR_FREE(xml); *ret_def = def; *ret_header = header; @@ -5660,7 +5664,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, else if (flags VIR_DOMAIN_SAVE_PAUSED) state = 0; -fd = qemuDomainSaveImageOpen(driver, path, def, header, +fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL, (flags VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0, wrapperFd, dxml, state, false, false); if (fd 0) @@ -5721,8 +5725,8 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path, /* We only take subset of virDomainDefFormat flags. */ virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL); -fd = qemuDomainSaveImageOpen(driver, path, def, header, false, NULL, - NULL, -1, false, false); +fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL, + false, NULL, NULL, -1, false, false); if (fd 0) goto cleanup; @@ -5759,8 +5763,8 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, else if (flags VIR_DOMAIN_SAVE_PAUSED) state = 0; -fd = qemuDomainSaveImageOpen(driver, path, def, header, false, NULL, - dxml, state, true, false); +fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL, + false, NULL, dxml, state, true, false); if (fd 0) { /* Check for special case of no change needed. */ @@ -5824,7 +5828,7 @@ qemuDomainObjRestore(virConnectPtr conn, virQEMUSaveHeader header; virFileWrapperFdPtr wrapperFd = NULL; -fd = qemuDomainSaveImageOpen(driver, path, def, header, +fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL, bypass_cache, wrapperFd, NULL, -1, false, true); if (fd 0) { -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] qemu: save image: Split out user provided XML checker
Extract code used to check save image XMLs provided by users to separate use. --- src/qemu/qemu_driver.c | 100 - 1 file changed, 66 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 15ad64d..e41a08e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5311,6 +5311,68 @@ static int qemuNodeGetSecurityModel(virConnectPtr conn, return ret; } + +/** + * qemuDomainSaveImageUpdateDef: + * @driver: qemu driver data + * @def: def of the domain from the save image + * @newxml: user provided replacement XML + * + * Returns the new domain definition in case @newxml is ABI compatible with the + * guest. + */ +static virDomainDefPtr +qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver, + virDomainDefPtr def, + const char *newxml) +{ +virDomainDefPtr ret = NULL; +virDomainDefPtr newdef_migr = NULL; +virDomainDefPtr newdef = NULL; +virCapsPtr caps = NULL; + +if (!(caps = virQEMUDriverGetCapabilities(driver, false))) +goto cleanup; + +if (!(newdef = virDomainDefParseString(newxml, caps, driver-xmlopt, + QEMU_EXPECTED_VIRT_TYPES, + VIR_DOMAIN_XML_INACTIVE))) +goto cleanup; + +if (!(newdef_migr = qemuDomainDefCopy(driver, + newdef, + VIR_DOMAIN_XML_MIGRATABLE))) +goto cleanup; + +if (!virDomainDefCheckABIStability(def, newdef_migr)) { +virResetLastError(); + +/* Due to a bug in older version of external snapshot creation + * code, the XML saved in the save image was not a migratable + * XML. To ensure backwards compatibility with the change of the + * saved XML type, we need to check the ABI compatibility against + * the user provided XML if the check against the migratable XML + * fails. Snapshots created prior to v1.1.3 have this issue. */ +if (!virDomainDefCheckABIStability(def, newdef)) +goto cleanup; + +/* use the user provided XML */ +ret = newdef; +newdef = NULL; +} else { +ret = newdef_migr; +newdef_migr = NULL; +} + + cleanup: +virObjectUnref(caps); +virDomainDefFree(newdef); +virDomainDefFree(newdef_migr); + +return ret; +} + + /* Return -1 on most failures after raising error, -2 if edit was specified * but xmlin and state (-1 for no change, 0 for paused, 1 for running) do * not represent any changes (no error raised), -3 if corrupt image was @@ -5431,45 +5493,15 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, QEMU_EXPECTED_VIRT_TYPES, VIR_DOMAIN_XML_INACTIVE))) goto error; -if (xmlin) { -virDomainDefPtr def2 = NULL; -virDomainDefPtr newdef = NULL; -if (!(def2 = virDomainDefParseString(xmlin, caps, driver-xmlopt, - QEMU_EXPECTED_VIRT_TYPES, - VIR_DOMAIN_XML_INACTIVE))) -goto error; +if (xmlin) { +virDomainDefPtr tmp; -newdef = qemuDomainDefCopy(driver, def2, VIR_DOMAIN_XML_MIGRATABLE); -if (!newdef) { -virDomainDefFree(def2); +if (!(tmp = qemuDomainSaveImageUpdateDef(driver, def, xmlin))) goto error; -} - -if (!virDomainDefCheckABIStability(def, newdef)) { -virDomainDefFree(newdef); -virResetLastError(); - -/* Due to a bug in older version of external snapshot creation - * code, the XML saved in the save image was not a migratable - * XML. To ensure backwards compatibility with the change of the - * saved XML type, we need to check the ABI compatibility against - * the user provided XML if the check against the migratable XML - * fails. Snapshots created prior to v1.1.3 have this issue. */ -if (!virDomainDefCheckABIStability(def, def2)) { -virDomainDefFree(def2); -goto error; -} - -/* use the user provided XML */ -newdef = def2; -def2 = NULL; -} else { -virDomainDefFree(def2); -} virDomainDefFree(def); -def = newdef; +def = tmp; } VIR_FREE(xml); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] qemu: save image: Split out checks done only when editing the save img
Move them to the single corresponding function rather than having them in the common chunk of code. --- src/qemu/qemu_driver.c | 76 +++--- 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0151fd2..1d82e93 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5373,10 +5373,22 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver, } -/* Return -1 on most failures after raising error, -2 if edit was specified - * but xmlin and state (-1 for no change, 0 for paused, 1 for running) do - * not represent any changes (no error raised), -3 if corrupt image was - * unlinked (no error raised), and opened fd on success. */ +/** + * qemuDomainSaveImageOpen: + * @driver: qemu driver data + * @path: path of the save image + * @ret_def: returns domain definition created from the XML stored in the image + * @ret_header: returns structure filled with data from the image header + * @xmlout: returns the XML from the image file (may be NULL) + * @bypass_cache: bypass cache when opening the file + * @wrapperFd: returns the file wrapper structure + * @open_write: open the file for writing (for updates) + * @unlink_corrupt: remove the image file if it is corrupted + * + * Returns the opened fd of the save image file and fills the apropriate fields + * on success. On error returns -1 on most failures, -3 if corrupt image was + * unlinked (no error raised). + */ static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) qemuDomainSaveImageOpen(virQEMUDriverPtr driver, const char *path, @@ -5385,14 +5397,14 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, char **xmlout, bool bypass_cache, virFileWrapperFdPtr *wrapperFd, -const char *xmlin, int state, bool edit, +bool open_write, bool unlink_corrupt) { int fd = -1; virQEMUSaveHeader header; char *xml = NULL; virDomainDefPtr def = NULL; -int oflags = edit ? O_RDWR : O_RDONLY; +int oflags = open_write ? O_RDWR : O_RDONLY; virCapsPtr caps = NULL; if (bypass_cache) { @@ -5477,18 +5489,6 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, goto error; } -if (edit STREQ(xml, xmlin) -(state 0 || state == header.was_running)) { -VIR_FREE(xml); -if (VIR_CLOSE(fd) 0) { -virReportSystemError(errno, _(cannot close file: %s), path); -goto error; -} -return -2; -} -if (state = 0) -header.was_running = state; - /* Create a domain from this XML */ if (!(def = virDomainDefParseString(xml, caps, driver-xmlopt, QEMU_EXPECTED_VIRT_TYPES, @@ -5643,21 +5643,15 @@ qemuDomainRestoreFlags(virConnectPtr conn, int ret = -1; virQEMUSaveHeader header; virFileWrapperFdPtr wrapperFd = NULL; -int state = -1; virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | VIR_DOMAIN_SAVE_RUNNING | VIR_DOMAIN_SAVE_PAUSED, -1); -if (flags VIR_DOMAIN_SAVE_RUNNING) -state = 1; -else if (flags VIR_DOMAIN_SAVE_PAUSED) -state = 0; - fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL, (flags VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0, - wrapperFd, dxml, state, false, false); + wrapperFd, false, false); if (fd 0) goto cleanup; @@ -5680,6 +5674,11 @@ qemuDomainRestoreFlags(virConnectPtr conn, goto cleanup; def = NULL; +if (flags VIR_DOMAIN_SAVE_RUNNING) +header.was_running = 1; +else if (flags VIR_DOMAIN_SAVE_PAUSED) +header.was_running = 0; + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0) goto cleanup; @@ -5725,7 +5724,7 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path, virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL); fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL, - false, NULL, NULL, -1, false, false); + false, NULL, false, false); if (fd 0) goto cleanup; @@ -5763,22 +5762,30 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, else if (flags VIR_DOMAIN_SAVE_PAUSED) state = 0; -fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL, - false, NULL, dxml, state, true, false); +fd = qemuDomainSaveImageOpen(driver, path, def, header, xml, + false, NULL, true, false); -if (fd 0) { -/* Check for special case of no change needed. */ -if (fd == -2) -ret = 0; +if (fd 0) goto cleanup; -} if
[libvirt] [PATCH 5/5] qemu: hook: Provide hook when restoring a domain save image
--- docs/hooks.html.in | 11 src/qemu/qemu_driver.c | 69 +- src/util/virhook.c | 3 ++- src/util/virhook.h | 1 + 4 files changed, 76 insertions(+), 8 deletions(-) diff --git a/docs/hooks.html.in b/docs/hooks.html.in index 07b9d49..265dbb7 100644 --- a/docs/hooks.html.in +++ b/docs/hooks.html.in @@ -177,6 +177,17 @@ script returns failure or the output XML is not valid, incoming migration will be canceled. This hook may be used, e.g., to change location of disk images for incoming domains./li + lispan class=sinceSince 1.2.9/span, the qemu hook script is +also called when restoring a saved image either via the API or +automatically when restoring a managed save machine. It is called +as: pre/etc/libvirt/hooks/qemu guest_name restore begin -/pre +with domain XML sent to standard input of the script. In this case, +the script acts as a filter and is supposed to modify the domain +XML and print it out on its standard output. Empty output is +identical to copying the input XML without changing it. In case the +script returns failure or the output XML is not valid, restore of the +image will be aborted. This hook may be used, e.g., to change +location of disk images for incoming domains./li lispan class=sinceSince 0.9.13/span, the qemu hook script is also called when the libvirtd daemon restarts and reconnects to previously running QEMU processes. If the script fails, the diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1d82e93..2dd2e48 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5636,20 +5636,23 @@ qemuDomainRestoreFlags(virConnectPtr conn, unsigned int flags) { virQEMUDriverPtr driver = conn-privateData; +qemuDomainObjPrivatePtr priv = NULL; virDomainDefPtr def = NULL; -virDomainDefPtr newdef = NULL; virDomainObjPtr vm = NULL; +char *xml = NULL; +char *xmlout = NULL; int fd = -1; int ret = -1; virQEMUSaveHeader header; virFileWrapperFdPtr wrapperFd = NULL; +bool hook_taint = false; virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | VIR_DOMAIN_SAVE_RUNNING | VIR_DOMAIN_SAVE_PAUSED, -1); -fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL, +fd = qemuDomainSaveImageOpen(driver, path, def, header, xml, (flags VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0, wrapperFd, false, false); if (fd 0) @@ -5658,12 +5661,29 @@ qemuDomainRestoreFlags(virConnectPtr conn, if (virDomainRestoreFlagsEnsureACL(conn, def) 0) goto cleanup; -if (dxml) { -if (!(newdef = qemuDomainSaveImageUpdateDef(driver, def, dxml))) +if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { +int hookret; + +if ((hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, def-name, + VIR_HOOK_QEMU_OP_RESTORE, + VIR_HOOK_SUBOP_BEGIN, + NULL, + dxml ? dxml : xml, + xmlout)) 0) goto cleanup; -virDomainDefFree(def); -def = newdef; +if (hookret == 0 xmlout) { +virDomainDefPtr tmp; + +VIR_DEBUG(Using hook-filtered domain XML: %s, xmlout); + +if (!(tmp = qemuDomainSaveImageUpdateDef(driver, def, xmlout))) +goto cleanup; + +virDomainDefFree(def); +def = tmp; +hook_taint = true; +} } if (!(vm = virDomainObjListAdd(driver-domains, def, @@ -5679,6 +5699,11 @@ qemuDomainRestoreFlags(virConnectPtr conn, else if (flags VIR_DOMAIN_SAVE_PAUSED) header.was_running = 0; +if (hook_taint) { +priv = vm-privateData; +priv-hookRun = true; +} + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0) goto cleanup; @@ -5697,6 +5722,8 @@ qemuDomainRestoreFlags(virConnectPtr conn, cleanup: virDomainDefFree(def); VIR_FORCE_CLOSE(fd); +VIR_FREE(xml); +VIR_FREE(xmlout); virFileWrapperFdFree(wrapperFd); if (vm) virObjectUnlock(vm); @@ -5834,12 +5861,15 @@ qemuDomainObjRestore(virConnectPtr conn, bool bypass_cache) { virDomainDefPtr def = NULL; +qemuDomainObjPrivatePtr priv = vm-privateData; int fd = -1; int ret = -1; +char *xml = NULL; +char *xmlout = NULL; virQEMUSaveHeader header; virFileWrapperFdPtr wrapperFd = NULL; -fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL, +fd = qemuDomainSaveImageOpen(driver, path, def, header, xml, bypass_cache, wrapperFd, false, true); if (fd 0) { if
Re: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On 09/17/2014 08:57 AM, Daniel P. Berrange wrote: + blt;driver csum='off' gso='off' guest_tso4='off' guest_tso6='off' guest_ecn='off'/gt;/b +lt;/interfacegt; Are we stuck with names with underscores in our XML? I'm still not sure if we've come up with the best naming for exposing all these knobs. I'm not really convinced having a 'guest_' prefix really buys us anything here, since there's no naming clash to avoid. Why don't we just kill the 'guest_' prefixes. NB, remember that precisely matching QEMU naming is a non-goal, we should be designing something that makes sense in general. I agree; I'd be fine with: driver csum='off' gso='off' tso4='off' tso6='off' ecn='off'/ with no need for a guest sub-structure. Yeah, we'll have to do some glue logic to translate to qemu names, but that's what libvirt is for. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] domaincapstest: Run cleanly on systems missing OVMF firmware
As of f05b6a918e28 the test produces the list of paths that can be passed to loader/ and libvirt knows about them. However, during the process of generating the list the paths are checked for their presence. This may produce different results on different systems. Therefore, the path - if missing - is added to pretend it's there. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/domaincapstest.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 8543963..067ad4d 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -118,6 +118,17 @@ fillQemuCaps(virDomainCapsPtr domCaps, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO); + +/* Moreover, as of f05b6a918e28 we are expecting to see + * OVMF_CODE.fd file which may not exists everywhere. */ +if (!domCaps-os.loader.values.nvalues) { +virDomainCapsLoaderPtr loader = domCaps-os.loader; + +if (fillStringValues(loader-values, + /usr/share/OVMF/OVMF_CODE.fd, + NULL) 0) +return -1; +} return 0; } #endif /* WITH_QEMU */ -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On 09/17/2014 04:57 PM, Daniel P. Berrange wrote: On Mon, Sep 15, 2014 at 04:30:46PM -0600, Eric Blake wrote: On 09/11/2014 05:43 AM, Ján Tomko wrote: Add the following attributes: csum, gso, guest_tso4, guest_tso6, guest_ecn to the driver element of network interface which control the virtio-net device properties of the same names. --- docs/formatdomain.html.in | 27 docs/schemas/domaincommon.rng | 25 +++ src/conf/domain_conf.c | 81 ++ src/conf/domain_conf.h | 5 ++ .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 + tests/qemuxml2xmltest.c| 1 + 6 files changed, 171 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a2ea758..5b2758a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null lt;model type='virtio'/gt; blt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5'/gt;/b lt;/interfacegt; +lt;interface type='network'gt; + lt;source network='default'/gt; + lt;target dev='vnet2'/gt; + lt;model type='virtio'/gt; + blt;driver csum='off' gso='off' guest_tso4='off' guest_tso6='off' guest_ecn='off'/gt;/b +lt;/interfacegt; Are we stuck with names with underscores in our XML? I'm still not sure if we've come up with the best naming for exposing all these knobs. I'm not really convinced having a 'guest_' prefix really buys us anything here, since there's no naming clash to avoid. Why don't we just kill the 'guest_' prefixes. The clash is in the options I didn't expose: http://git.qemu.org/?p=qemu.git;a=blob;f=include/hw/virtio/virtio-net.h;h=6ceb5aa92 because they weren't requested by the (private :() bug 1139364 Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domaincapstest: Run cleanly on systems missing OVMF firmware
On Wed, Sep 17, 2014 at 05:32:03PM +0200, Michal Privoznik wrote: As of f05b6a918e28 the test produces the list of paths that can be passed to loader/ and libvirt knows about them. However, during the process of generating the list the paths are checked for their presence. This may produce different results on different systems. Therefore, the path - if missing - is added to pretend it's there. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/domaincapstest.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 8543963..067ad4d 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -118,6 +118,17 @@ fillQemuCaps(virDomainCapsPtr domCaps, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO); + +/* Moreover, as of f05b6a918e28 we are expecting to see + * OVMF_CODE.fd file which may not exists everywhere. */ +if (!domCaps-os.loader.values.nvalues) { +virDomainCapsLoaderPtr loader = domCaps-os.loader; + +if (fillStringValues(loader-values, + /usr/share/OVMF/OVMF_CODE.fd, + NULL) 0) +return -1; +} return 0; } #endif /* WITH_QEMU */ -- 1.8.5.5 ACK, build-breaker (at least for me). Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domaincapstest: Run cleanly on systems missing OVMF firmware
On 17.09.2014 17:40, Martin Kletzander wrote: On Wed, Sep 17, 2014 at 05:32:03PM +0200, Michal Privoznik wrote: As of f05b6a918e28 the test produces the list of paths that can be passed to loader/ and libvirt knows about them. However, during the process of generating the list the paths are checked for their presence. This may produce different results on different systems. Therefore, the path - if missing - is added to pretend it's there. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/domaincapstest.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 8543963..067ad4d 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -118,6 +118,17 @@ fillQemuCaps(virDomainCapsPtr domCaps, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO); + +/* Moreover, as of f05b6a918e28 we are expecting to see + * OVMF_CODE.fd file which may not exists everywhere. */ +if (!domCaps-os.loader.values.nvalues) { +virDomainCapsLoaderPtr loader = domCaps-os.loader; + +if (fillStringValues(loader-values, + /usr/share/OVMF/OVMF_CODE.fd, + NULL) 0) +return -1; +} return 0; } #endif /* WITH_QEMU */ -- 1.8.5.5 ACK, build-breaker (at least for me). Thank you, pushed now. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/2] Use huge pages on UMA guests widely
On Mon, Sep 15, 2014 at 01:48:05PM +0200, Michal Privoznik wrote: *** BLURB HERE *** Michal Privoznik (2): conf: Disallow nonexistent NUMA nodes for hugepages qemu: Honor hugepages for UMA domains src/qemu/qemu_command.c| 49 -- .../qemuxml2argv-hugepages-pages4.xml | 45 .../qemuxml2argv-hugepages-pages5.args | 7 .../qemuxml2argv-hugepages-pages5.xml | 32 ++ tests/qemuxml2argvtest.c | 3 ++ 5 files changed, 132 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.xml ACK series, Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On Wed, Sep 17, 2014 at 05:36:18PM +0200, Ján Tomko wrote: On 09/17/2014 04:57 PM, Daniel P. Berrange wrote: On Mon, Sep 15, 2014 at 04:30:46PM -0600, Eric Blake wrote: On 09/11/2014 05:43 AM, Ján Tomko wrote: Add the following attributes: csum, gso, guest_tso4, guest_tso6, guest_ecn to the driver element of network interface which control the virtio-net device properties of the same names. --- docs/formatdomain.html.in | 27 docs/schemas/domaincommon.rng | 25 +++ src/conf/domain_conf.c | 81 ++ src/conf/domain_conf.h | 5 ++ .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 + tests/qemuxml2xmltest.c| 1 + 6 files changed, 171 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a2ea758..5b2758a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null lt;model type='virtio'/gt; blt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5'/gt;/b lt;/interfacegt; +lt;interface type='network'gt; + lt;source network='default'/gt; + lt;target dev='vnet2'/gt; + lt;model type='virtio'/gt; + blt;driver csum='off' gso='off' guest_tso4='off' guest_tso6='off' guest_ecn='off'/gt;/b +lt;/interfacegt; Are we stuck with names with underscores in our XML? I'm still not sure if we've come up with the best naming for exposing all these knobs. I'm not really convinced having a 'guest_' prefix really buys us anything here, since there's no naming clash to avoid. Why don't we just kill the 'guest_' prefixes. The clash is in the options I didn't expose: http://git.qemu.org/?p=qemu.git;a=blob;f=include/hw/virtio/virtio-net.h;h=6ceb5aa92 because they weren't requested by the (private :() bug 1139364 Ah, so this is why you shouldn't take the precise solution requested in a bug too literally, and instead look at the general picture :-) So QEMU exposes alot of stuff: $ qemu-kvm -device virtio-net,? virtio-net-pci.ioeventfd=on/off virtio-net-pci.vectors=uint32 virtio-net-pci.indirect_desc=on/off virtio-net-pci.event_idx=on/off virtio-net-pci.any_layout=on/off virtio-net-pci.csum=on/off virtio-net-pci.guest_csum=on/off virtio-net-pci.gso=on/off virtio-net-pci.guest_tso4=on/off virtio-net-pci.guest_tso6=on/off virtio-net-pci.guest_ecn=on/off virtio-net-pci.guest_ufo=on/off virtio-net-pci.host_tso4=on/off virtio-net-pci.host_tso6=on/off virtio-net-pci.host_ecn=on/off virtio-net-pci.host_ufo=on/off virtio-net-pci.mrg_rxbuf=on/off virtio-net-pci.status=on/off virtio-net-pci.ctrl_vq=on/off virtio-net-pci.ctrl_rx=on/off virtio-net-pci.ctrl_vlan=on/off virtio-net-pci.ctrl_rx_extra=on/off virtio-net-pci.ctrl_mac_addr=on/off virtio-net-pci.ctrl_guest_offloads=on/off virtio-net-pci.mq=on/off virtio-net-pci.mac=macaddr virtio-net-pci.vlan=vlan virtio-net-pci.netdev=netdev virtio-net-pci.bootindex=int32 virtio-net-pci.x-txtimer=uint32 virtio-net-pci.x-txburst=int32 virtio-net-pci.tx=str virtio-net-pci.addr=pci-devfn virtio-net-pci.romfile=str virtio-net-pci.rombar=uint32 virtio-net-pci.multifunction=on/off virtio-net-pci.command_serr_enable=on/off Which to me indicates that Eric's suggestion for sub-elements is a good idea. eg go for: driver guest / host / /driver and support the host bits too Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On 09/17/2014 09:52 AM, Daniel P. Berrange wrote: Ah, so this is why you shouldn't take the precise solution requested in a bug too literally, and instead look at the general picture :-) So QEMU exposes alot of stuff: $ qemu-kvm -device virtio-net,? virtio-net-pci.guest_tso4=on/off virtio-net-pci.guest_tso6=on/off virtio-net-pci.guest_ecn=on/off virtio-net-pci.guest_ufo=on/off virtio-net-pci.host_tso4=on/off virtio-net-pci.host_tso6=on/off virtio-net-pci.host_ecn=on/off virtio-net-pci.host_ufo=on/off Which to me indicates that Eric's suggestion for sub-elements is a Wasn't my idea, but Jan's. good idea. eg go for: driver guest / host / /driver and support the host bits too but yes, I could live with that, now that I'm seeing how many more options there are to be exposed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv6 08/11] qemu: bulk stats: add block allocation information
On 09/17/14 00:37, Eric Blake wrote: On 09/15/2014 09:42 AM, Peter Krempa wrote: From: Francesco Romani from...@redhat.com Management software wants to be able to allocate disk space on demand. To support this they need keep track of the space occupation of the block device. This information is reported by qemu as part of block stats. This patch extend the block information in the bulk stats with the allocation information. To keep the same behaviour a helper is extracted from qemuMonitorJSONGetBlockExtent in order to get per-device allocation information. Signed-off-by: Francesco Romani from...@redhat.com --- src/libvirt.c| 2 + src/qemu/qemu_driver.c | 18 + src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 91 ++-- 4 files changed, 92 insertions(+), 20 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index ab10a3a..a8892a2 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21654,6 +21654,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * unsigned long long. * block.num.errors - Xen only: the 'oo_req' value as *unsigned long long. + * block.num.allocation - offset of the highest written sector + *as unsigned long long. If we are repeating virDomainGetBlockInfo() here, I'd rather see us report all 3 pieces of information (allocation, capacity, physical) instead of just one. +typedef enum { +QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK, +QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT, +QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS, +QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET +} qemuMonitorBlockExtentError; We require C99, so please have a trailing comma in the enum (it makes adding new elements easier, because they don't have to modify an existing line). Thanks for the reviews, I've pushed this series with the changes you've suggested except this patch as either I or Francesco will add the requested fields. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] CVE-2014-3633: qemu: blkiotune: Use correct definition when looking up disk
Live definition was used to look up the disk index while persistent one was indexed leading to a crash in qemuDomainGetBlockIoTune. Use the correct def and report a nice error. Unfortunately it's accessible via read-only connection. Introduced in: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa (v0.9.8) Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140724 Reported-by: Luyao Huang lhu...@redhat.com Signed-off-by: Peter Krempa pkre...@redhat.com --- src/qemu/qemu_driver.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a5a49ac..209c40e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16317,9 +16317,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, } if (flags VIR_DOMAIN_AFFECT_CONFIG) { -int idx = virDomainDiskIndexByName(vm-def, disk, true); -if (idx 0) +int idx = virDomainDiskIndexByName(persistentDef, disk, true); +if (idx 0) { +virReportError(VIR_ERR_INVALID_ARG, + _(disk '%s' was not found in the domain config), + disk); goto endjob; +} reply = persistentDef-disks[idx]-blkdeviotune; } -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] CVE-2014-3633: qemu: blkiotune: Use correct definition when looking up disk
On 09/17/2014 10:25 AM, Peter Krempa wrote: Live definition was used to look up the disk index while persistent one was indexed leading to a crash in qemuDomainGetBlockIoTune. Use the correct def and report a nice error. Unfortunately it's accessible via read-only connection. Mitigation - a read-only connection can only crash libvirtd in the cases where the guest is hot-plugging disks without reflecting those changes to the persistent definition. So avoiding hotplug, or doing hotplug where persistent is always modified alongside live definition, will avoid the out-of-bounds access. Introduced in: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa (v0.9.8) Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140724 Reported-by: Luyao Huang lhu...@redhat.com Signed-off-by: Peter Krempa pkre...@redhat.com --- src/qemu/qemu_driver.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) ACK. I can write up the libvirt security notice; we'll eventually need this backported to all the affected maint branches. I'll coordinate the backport effort with you on IRC. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a5a49ac..209c40e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16317,9 +16317,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, } if (flags VIR_DOMAIN_AFFECT_CONFIG) { -int idx = virDomainDiskIndexByName(vm-def, disk, true); -if (idx 0) +int idx = virDomainDiskIndexByName(persistentDef, disk, true); +if (idx 0) { +virReportError(VIR_ERR_INVALID_ARG, + _(disk '%s' was not found in the domain config), + disk); goto endjob; +} reply = persistentDef-disks[idx]-blkdeviotune; } -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] CVE-2014-3633: qemu: blkiotune: Use correct definition when looking up disk
On 09/17/14 18:31, Eric Blake wrote: On 09/17/2014 10:25 AM, Peter Krempa wrote: Live definition was used to look up the disk index while persistent one was indexed leading to a crash in qemuDomainGetBlockIoTune. Use the correct def and report a nice error. Unfortunately it's accessible via read-only connection. Mitigation - a read-only connection can only crash libvirtd in the cases where the guest is hot-plugging disks without reflecting those changes to the persistent definition. So avoiding hotplug, or doing hotplug where persistent is always modified alongside live definition, will avoid the out-of-bounds access. I've added this paragraph to the commit message Introduced in: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa (v0.9.8) Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140724 Reported-by: Luyao Huang lhu...@redhat.com Signed-off-by: Peter Krempa pkre...@redhat.com --- src/qemu/qemu_driver.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) ACK. I can write up the libvirt security notice; we'll eventually need this backported to all the affected maint branches. I'll coordinate the backport effort with you on IRC. and pushed this patch. I can handle the backport tomorrow if you don't beat me to it. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] maint: clean up _virDomainBlockStats
On 09/16/2014 07:19 AM, James wrote: I clean up all _virDomainBlockStats. Signed-off-by: James james.wangyu...@huawei.com Signed-off-by: Wang Rui moon.wang...@huawei.com --- src/driver.h | 2 +- src/libvirt.c| 2 +- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 2 +- src/xen/block_stats.c| 4 ++-- src/xen/block_stats.h| 2 +- src/xen/xen_driver.c | 2 +- src/xen/xen_hypervisor.c | 2 +- src/xen/xen_hypervisor.h | 2 +- tools/virsh-domain-monitor.c | 2 +- 11 files changed, 12 insertions(+), 12 deletions(-) ACK; I'll touch up the attribution in the same manner as 1/3 and push shortly. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] maint: clean up _virDomainMemoryStat
On 09/16/2014 07:19 AM, James wrote: I clean up all _virDomainMemoryStat. Signed-off-by: James james.wangyu...@huawei.com Signed-off-by: Wang Rui moon.wang...@huawei.com --- daemon/remote.c | 2 +- src/driver.h | 2 +- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_monitor_text.c | 2 +- src/remote/remote_driver.c | 2 +- tools/virsh-domain-monitor.c | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) ACK; same story about the commit cleanup. I'd also like to push a .mailmap entry to consolidate your prior entries; am I correct that all of these are from you? $ git shortlog --author=james.wangyu...@huawei.com James (1): util: virTimeFieldsThenRaw never returns negative Wang Yufei (4): qemu: Avoid assigning unavailable migration ports docs: fix virDomainRestoreFlags description bug docs: fix double articles bug cgroup: Fix start VMs coincidently failed Wangyufei (A) (1): docs: delete extra character Wangyufei (James) (1): qemuAgentDispose: Reset lastError diff --git a/daemon/remote.c b/daemon/remote.c index 0ea2815..daa4b60 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1604,7 +1604,7 @@ remoteDispatchDomainMemoryStats(virNetServerPtr server ATTRIBUTE_UNUSED, remote_domain_memory_stats_ret *ret) { virDomainPtr dom = NULL; -struct _virDomainMemoryStat *stats = NULL; +virDomainMemoryStatPtr stats = NULL; int nr_stats; size_t i; int rv = -1; diff --git a/src/driver.h b/src/driver.h index 76142bd..bb748c4 100644 --- a/src/driver.h +++ b/src/driver.h @@ -535,7 +535,7 @@ typedef int typedef int (*virDrvDomainMemoryStats)(virDomainPtr domain, - struct _virDomainMemoryStat *stats, + virDomainMemoryStatPtr stats, unsigned int nr_stats, unsigned int flags); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 8ab4cf2..c3cd62c 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5387,7 +5387,7 @@ lxcNodeGetInfo(virConnectPtr conn, static int lxcDomainMemoryStats(virDomainPtr dom, - struct _virDomainMemoryStat *stats, + virDomainMemoryStatPtr stats, unsigned int nr_stats, unsigned int flags) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9f5c977..59a4b3c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10178,7 +10178,7 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom, static int qemuDomainMemoryStats(virDomainPtr dom, - struct _virDomainMemoryStat *stats, + virDomainMemoryStatPtr stats, unsigned int nr_stats, unsigned int flags) { diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 2bc8261..46d2782 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -684,7 +684,7 @@ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon, if ((offset = strstr(reply, BALLOON_PREFIX)) != NULL) { offset += strlen(BALLOON_PREFIX); -struct _virDomainMemoryStat stats[1]; +virDomainMemoryStatStruct stats[1]; if (qemuMonitorParseBalloonInfo(offset, stats, 1) == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 4a1b04b..75a3a7b 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2690,7 +2690,7 @@ remoteDomainGetSchedulerType(virDomainPtr domain, int *nparams) static int remoteDomainMemoryStats(virDomainPtr domain, -struct _virDomainMemoryStat *stats, +virDomainMemoryStatPtr stats, unsigned int nr_stats, unsigned int flags) { diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index b2e1fc8..4f6aaa3 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -295,7 +295,7 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; const char *name; -struct _virDomainMemoryStat stats[VIR_DOMAIN_MEMORY_STAT_NR]; +virDomainMemoryStatStruct stats[VIR_DOMAIN_MEMORY_STAT_NR]; unsigned int nr_stats; size_t i; int ret = false; -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Don't fail qemuProcessAttach for IOThreads if no JSON
While doing some investigation for another bug I found that I could not qemu-attach to the process and got the following: error: Operation not supported: JSON monitor is required while running thru qemuProcessAttach. Since we can only get the data using the JSON parser and if the guest to be attached to doesn't have it we shouldn't just fail. See example in virsh qemu-attach for sample command that failed. Signed-off-by: John Ferlan jfer...@redhat.com --- I also considered removing the call from qemuProcessAttach rather than this approach. src/qemu/qemu_monitor.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 8927dbb..4342088 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4112,11 +4112,9 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon, return -1; } -if (!mon-json) { -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, - _(JSON monitor is required)); -return -1; -} +/* Requires JSON to make the query */ +if (!mon-json) +return 0; return qemuMonitorJSONGetIOThreads(mon, iothreads); } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] qemu: Improve check for local storage
On 09/17/14 00:29, John Ferlan wrote: On 09/11/2014 01:47 PM, Peter Krempa wrote: Now that we have a simple function to check locality of storage, reuse it in qemuDomainCheckDiskPresence(). Also reuse check for empty storage source. --- src/qemu/qemu_domain.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) ACK Thanks, I've currently pushed 2/6 and 6/6 and I'll try to implement your feedback on the rest later and I'll possibly repost the series if I find the changes were not trivial. John Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Add rawio for hostdev
On 09/09/2014 07:40 PM, John Ferlan wrote: The rawio setting for a device.../ disk.../ lun isn't duplicated in the scsi_host hostdev device for a lun. Rather than requiring some disk in the disk list to add the rawio, add the rawio property to the scsi_host hostdev lun. John Ferlan (3): hostdev: Add rawio attribute to _virDomainHostdevSubsysSCSI qemu: Process the hostdev rawio setting domain_conf: Process the rawio for a hostdev device docs/formatdomain.html.in | 12 ++-- docs/schemas/domaincommon.rng | 18 +++ src/conf/domain_conf.c | 31 +++ src/conf/domain_conf.h | 2 ++ src/qemu/qemu_domain.c | 21 + src/qemu/qemu_domain.h | 4 +++ src/qemu/qemu_driver.c | 1 + src/qemu/qemu_process.c| 20 - .../qemuxml2argv-hostdev-scsi-rawio.xml| 35 ++ tests/qemuxml2xmltest.c| 1 + 10 files changed, 135 insertions(+), 10 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml ping? tks John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix crash with shared disks
On 09/17/2014 06:45 AM, Ján Tomko wrote: Commit f36a94f introduced a double free on all success paths in qemuSharedDeviceEntryInsert. Only call qemuSharedDeviceEntryFree on the error path and set entry to NULL before jumping there if the entry already is in the hash table. https://bugzilla.redhat.com/show_bug.cgi?id=1142722 --- src/qemu/qemu_conf.c | 26 -- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index ac10b64..adc6caf 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1011,38 +1011,36 @@ qemuSharedDeviceEntryInsert(virQEMUDriverPtr driver, const char *name) { qemuSharedDeviceEntry *entry = NULL; -int ret = -1; if ((entry = virHashLookup(driver-sharedDevices, key))) { /* Nothing to do if the shared scsi host device is already * recorded in the table. */ -if (qemuSharedDeviceEntryDomainExists(entry, name, NULL)) { -ret = 0; -goto cleanup; +if (!qemuSharedDeviceEntryDomainExists(entry, name, NULL)) { +if (VIR_EXPAND_N(entry-domains, entry-ref, 1) 0 || +VIR_STRDUP(entry-domains[entry-ref - 1], name) 0) { +/* entry is owned by the hash table here */ +entry = NULL; [1] Assigning to NULL causes an issue +goto error; +} } - -if (VIR_EXPAND_N(entry-domains, entry-ref, 1) 0 || -VIR_STRDUP(entry-domains[entry-ref - 1], name) 0) -goto cleanup; } else { if (VIR_ALLOC(entry) 0 || VIR_ALLOC_N(entry-domains, 1) 0 || VIR_STRDUP(entry-domains[0], name) 0) -goto cleanup; +goto error; entry-ref = 1; if (virHashAddEntry(driver-sharedDevices, key, entry)) -goto cleanup; +goto error; } -ret = 0; +return 0; - cleanup: + error: qemuSharedDeviceEntryFree(entry, NULL); [1] Because this is prototyped as: void qemuSharedDeviceEntryFree(void *payload, const void *name) ATTRIBUTE_NONNULL(1); Coverity gives us a warning when entry = NULL... It's solveable by either allowing NULL for the function or only calling if (entry) ACK as long as we handle in some manner. John - -return ret; +return -1; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] retiring v0.9.6-maint
Any objections to retiring the v0.9.6-maint branch? After all, we have already retired the v0.9.11-maint branch (http://libvirt.org/git/?p=libvirt.git;a=commit;h=cd0d348ed), and the only activity on v0.9.6-maint since 0.9.6.4 was released in January 2013 was the backport of a single CVE fix. The branch no longer builds cleanly on Fedora 20, and while I could identify patches to backport to fix the build situation, it's not worth my time if we can just retire the branch. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH-RFC-V2] qemu: Add network bandwidth setting for ethernet interfaces
V2: Consolidate calls to virNetDevBandwidthSet Clear bandwidth settings when the interface is detached or domain destroyed V1: Ethernet interfaces in libvirt currently do not support bandwidth setting. For example, following xml file for an interface will not apply these settings to corresponding qdiscs. interface type=ethernet mac address=02:36:1d:18:2a:e4/ model type=virtio/ script path=/ target dev=tap361d182a-e4/ bandwidth inbound average=984 peak=1024 burst=64/ outbound average=2000 peak=2048 burst=128/ /bandwidth /interface Signed-off-by: Anirban Chakraborty abc...@juniper.net --- src/conf/domain_conf.h | 7 +++ src/lxc/lxc_process.c | 27 ++- src/qemu/qemu_command.c | 9 - src/qemu/qemu_driver.c | 21 + src/qemu/qemu_hotplug.c | 13 + src/util/virnetdevmacvlan.c | 10 -- src/util/virnetdevmacvlan.h | 1 - 7 files changed, 59 insertions(+), 29 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 640a4c5..3c950f1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -829,6 +829,13 @@ typedef enum { VIR_DOMAIN_NET_TYPE_LAST } virDomainNetType; +/* check bandwidth configuration for a network interface */ +#define NETDEVIF_SUPPORT_BANDWIDTH(type) \ + ((type == VIR_DOMAIN_NET_TYPE_ETHERNET || \ + type == VIR_DOMAIN_NET_TYPE_NETWORK || \ + type == VIR_DOMAIN_NET_TYPE_BRIDGE || \ + type == VIR_DOMAIN_NET_TYPE_DIRECT) ? true : false) + /* the backend driver used for virtio interfaces */ typedef enum { VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT, /* prefer kernel, fall back to user */ diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index ed30c37..5ef91e8 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -274,11 +274,6 @@ char *virLXCProcessSetupInterfaceBridged(virConnectPtr conn, if (virNetDevSetOnline(parentVeth, true) 0) goto cleanup; -if (virNetDevBandwidthSet(net-ifname, - virDomainNetGetActualBandwidth(net), - false) 0) -goto cleanup; - if (net-filter virDomainConfNWFilterInstantiate(conn, vm-uuid, net) 0) goto cleanup; @@ -300,6 +295,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn, virNetDevBandwidthPtr bw; virNetDevVPortProfilePtr prof; virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); +const char *linkdev = virDomainNetGetActualDirectDev(net); /* XXX how todo bandwidth controls ? * Since the 'net-ifname' is about to be moved to a different @@ -329,16 +325,15 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn, if (virNetDevMacVLanCreateWithVPortProfile( net-ifname, net-mac, -virDomainNetGetActualDirectDev(net), +linkdev, virDomainNetGetActualDirectMode(net), false, def-uuid, -virDomainNetGetActualVirtPortProfile(net), +prof, res_ifname, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, cfg-stateDir, -virDomainNetGetActualBandwidth(net), 0) 0) +0) 0) goto cleanup; - ret = res_ifname; cleanup: @@ -368,6 +363,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, int ret = -1; size_t i; size_t niface = 0; +int actualType; for (i = 0; i def-nnets; i++) { char *veth = NULL; @@ -381,7 +377,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, if (VIR_EXPAND_N(*veths, *nveths, 1) 0) goto cleanup; -switch (virDomainNetGetActualType(def-nets[i])) { +actualType = virDomainNetGetActualType(def-nets[i]); +switch (actualType) { case VIR_DOMAIN_NET_TYPE_NETWORK: { virNetworkPtr network; char *brname = NULL; @@ -444,11 +441,15 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, case VIR_DOMAIN_NET_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _(Unsupported network type %s), - virDomainNetTypeToString( - virDomainNetGetActualType(def-nets[i]) - )); + virDomainNetTypeToString(actualType)); goto cleanup; } +/* set network bandwidth */ +if (NETDEVIF_SUPPORT_BANDWIDTH(actualType) virNetDevBandwidthSet( +def-nets[i]-ifname, virDomainNetGetActualBandwidth( +def-nets[i]), +false) 0) + goto cleanup; (*veths)[(*nveths)-1] = veth; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f2e6e5a..404c51b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -191,7 +191,6 @@