- Devel - libvirt List Archives

[libvirt] [REPOST PATCHv2 0/6] Converge Storage Pool vHBA logic with Node Device
by John Ferlan 10 Mar '17

10 Mar '17

Repost to merge in recent changes (thru commit id 'dec6d9df') v1: http://www.redhat.com/archives/libvir-list/2017-February/msg00963.html Changes since v1: NB: Former Patch 1 - was pushed Patch 1 - Was the old patch 2... Similar for rest of series... Altered to match the adjusted testNodeDeviceMockCreateVport from commit 2a862092 Patch 2 - reworked the code to not place the new module in src/util, but rather in src/conf as storage_adapter_conf.{c,h}. The concepts didn't change - only the name/location. Removed the changes for util/virvirscsi.c since virSCSIHostGetNameByParentaddr wouldn't be allowed to use virStorageAdapterSCSIHostPtr since it's a conf definition. Patch 3 - Unchanged, ACK'd in v1 Patch 4 - Unchanged, ACK'd in v1 Patch 5 - Unchanged... Even though there is concern over usage of API's in this manner (e.g. calling virNodeDeviceLookupSCSIHostByWWN as well as virNodeDeviceCreateXML) - the "purpose" of the exercise is a means to have the StoragePool code use the same algorithms as the node device code instead of what could be a potentially long slow journey through the sysfs file system. The key piece being the replication of the API's that find the parent in the node device object list. I suppose it would be possible to add even more API's to get that answer, but I'm not sure it'd be worthwhile at this point. As for the concern about deadlock - I understand it, but since the whole purpose of the API is to essentially utilize the nodedev create algorithm there'd be no reason for nodedev to call it. If it doesn't get ACK'd - that's OK - I can hold onto it and reconsider some other means at some point in time in the future (probably distant future too). Patch 6 - Unchanged The former patch 8 is dropped for now - needs some more "discussion" as to viability and/or need. John Ferlan (6): tests: Add createVHBAByStoragePool-by-parent to fchosttest conf: Convert virStoragePoolSourceAdapter to virStorageAdapter util: Rename virFileWaitForDevices conf: Move/rename createVport and deleteVport util: Alter virNodeDevice{Create|Delete}Vport to use nodedev APIs tests: Add more storage pool vHBA tests po/POTFILES.in | 1 + src/Makefile.am | 1 + src/conf/node_device_conf.c | 332 +++++++++++++++++++++++++++++++++ src/conf/node_device_conf.h | 9 + src/conf/storage_adapter_conf.c | 291 +++++++++++++++++++++++++++++ src/conf/storage_adapter_conf.h | 89 +++++++++ src/conf/storage_conf.c | 333 ++++++++-------------------------- src/conf/storage_conf.h | 35 +--- src/libvirt_private.syms | 20 +- src/node_device/node_device_driver.c | 2 +- src/phyp/phyp_driver.c | 3 +- src/storage/storage_backend_disk.c | 6 +- src/storage/storage_backend_iscsi.c | 2 +- src/storage/storage_backend_logical.c | 4 +- src/storage/storage_backend_mpath.c | 2 +- src/storage/storage_backend_scsi.c | 267 +++++---------------------- src/storage/storage_util.c | 2 +- src/test/test_driver.c | 99 +++++++++- src/util/virfile.h | 2 - src/util/virutil.c | 4 +- src/util/virutil.h | 2 + tests/fchosttest.c | 111 ++++++++++++ 22 files changed, 1087 insertions(+), 530 deletions(-) create mode 100644 src/conf/storage_adapter_conf.c create mode 100644 src/conf/storage_adapter_conf.h -- 2.9.3

2 9

[libvirt] [PATCH for-2.9 v3 0/3] Use non-blacklisted family/model/stepping for Haswell CPU model
by Eduardo Habkost 10 Mar '17

10 Mar '17

I plan to merge this bug fix into 2.9. Changes v2 -> v3: * Rebase to latest master * Don't touch max_x86_cpu_initfn() to reduce risk post hard freeze Changes v1 -> v2: * Coding style fixes * Make series simpler: * Don't use trick: char vendor[static (CPUID_VENDOR_SZ + 1)] because it confuses checkpatch.pl * Removed patch "Add explicit array size to x86_cpu_vendor_words2str()" * Rebased on top of my x86-next branch: https://github.com/ehabkost/qemu x86-next Git branch for testing: https://github.com/ehabkost/qemu-hacks work/x86-rtm-blacklist Diff from v1: diff --git a/target/i386/cpu.h b/target/i386/cpu.h index cd94726e43..647435a1d9 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1431,7 +1431,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, void cpu_clear_apic_feature(CPUX86State *env); void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx); -void host_vendor_fms(char vendor[static (CPUID_VENDOR_SZ + 1)], int *family, int *model, int *stepping); +void host_vendor_fms(char *vendor, int *family, int *model, int *stepping); /* helper.c */ int x86_cpu_handle_mmu_fault(CPUState *cpu, vaddr addr, diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 25c6c5e115..eab1ad7935 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -682,7 +682,7 @@ void host_cpuid(uint32_t function, uint32_t count, *edx = vec[3]; } -void host_vendor_fms(char vendor[static (CPUID_VENDOR_SZ + 1)], int *family, int *model, int *stepping) +void host_vendor_fms(char *vendor, int *family, int *model, int *stepping) { uint32_t eax, ebx, ecx, edx; @@ -1570,7 +1570,8 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data) xcc->kvm_required = true; xcc->ordering = 9; - host_vendor_fms(host_cpudef.vendor, &host_cpudef.family, &host_cpudef.model, &host_cpudef.stepping); + host_vendor_fms(host_cpudef.vendor, &host_cpudef.family, + &host_cpudef.model, &host_cpudef.stepping); cpu_x86_fill_model_id(host_cpudef.model_id); --- A recent glibc commit[1] added a blacklist to ensure it won't use TSX on hosts that are known to have a broken TSX implementation. Our existing Haswell CPU model has a blacklisted family/model/stepping combination, so it has to be updated to make sure guests will really use TSX. This is done by patch 5/5. However, to do this safely we need to ensure the host CPU is not a blacklisted one, so we won't mislead guests by exposing known-to-be-good FMS values on a known-to-be-broken host. This is done by patch 3/5. [1] https://sourceware.org/git/?p=glibc.git;a=commit;h=2702856bf45c82cf8e69f206… --- Cc: dgilbert(a)redhat.com Cc: fweimer(a)redhat.com Cc: carlos(a)redhat.com Cc: triegel(a)redhat.com Cc: berrange(a)redhat.com Cc: jdenemar(a)redhat.com Cc: pbonzini(a)redhat.com Eduardo Habkost (3): i386: host_vendor_fms() helper function i386/kvm: Blacklist TSX on known broken hosts i386: Change stepping of Haswell to non-blacklisted value include/hw/i386/pc.h | 5 +++++ target/i386/cpu.h | 1 + target/i386/cpu.c | 21 ++++++++++++++++++++- target/i386/kvm.c | 17 +++++++++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) -- 2.11.0.259.g40922b1

1 4

[libvirt] [PATCH v3] Document preferred naming conventions
by Daniel P. Berrange 10 Mar '17

10 Mar '17

This documents the preferred conventions for naming files, structs, enums, typedefs and functions. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- Changed in v3: - Clarify function naming wrt verb & subject - Simplify macro naming, since in practice libvirt code doesn't follow any rules consistently aside from having a VIR_ prefix. HACKING | 80 ++++++++++++++++++++++++++++++++++++++++++++ docs/hacking.html.in | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++ docs/hacking2.xsl | 4 +++ 3 files changed, 177 insertions(+) diff --git a/HACKING b/HACKING index fff003b..2c65985 100644 --- a/HACKING +++ b/HACKING @@ -239,6 +239,86 @@ on the subject, on Richard Jones' guide to working with open source projects <http://people.redhat.com/rjones/how-to-supply-code-to-open-source-projects/>. +Naming conventions +================== +When reading libvirt code, a number of different naming conventions will be +evident due to various changes in thinking over the course of the project's +lifetime. The conventions documented below should be followed when creating +any entirely new files in libvirt. When working on existing files, while it is +desirable to apply these conventions, keeping a consistent style with existing +code in that particular file is generally more important. The overall guiding +principal is that every file, enum, struct, function, macro and typedef name +must have a 'vir' or 'VIR' prefix. All local scope variable names are exempt, +and global variables are exempt, unless exported in a header file. + +*File names* + +File naming varies depending on the subdirectory. The preferred style is to +have a 'vir' prefix, followed by a name which matches the name of the +functions / objects inside the file. For example, a file containing an object +'virHashtable' is stored in files 'virhashtable.c' and 'virhashtable.h'. +Sometimes, methods which would otherwise be declared 'static' need to be +exported for use by a test suite. For this purpose a second header file should +be added with a suffix of 'priv'. e.g. 'virhashtablepriv.h'. Use of +underscores in file names is discouraged when using the 'vir' prefix style. +The 'vir' prefix naming applies to src/util, src/rpc and tests/ directories. +Most other directories do not follow this convention. + + + +*Enum type & field names* + +All enums should have a 'vir' prefix in their typedef name, and each following +word should have its first letter in uppercase. The enum name should match the +typedef name with a leading underscore. The enum member names should be in all +uppercase, and use an underscore to separate each word. The enum member name +prefix should match the enum typedef name. + + typedef enum _virSocketType virSocketType; + enum _virSocketType { + VIR_SOCKET_TYPE_IPV4, + VIR_SOCKET_TYPE_IPV6, + }; + + +*Struct type names* + +All structs should have a 'vir' prefix in their typedef name, and each +following word should have its first letter in uppercase. The struct name +should be the same as the typedef name with a leading underscore. A second +typedef should be given for a pointer to the struct with a 'Ptr' suffix. + + typedef struct _virHashTable virHashTable; + typedef virHashTable *virHashTablePtr; + struct _virHashTable { + ... + }; + + +*Function names* + +All functions should have a 'vir' prefix in their name, followed by one or +more words with first letter of each word capitalized. Underscores should not +be used in function names. If the function is operating on an object, then the +function name prefix should match the object typedef name, otherwise it should +match the filename. Following this comes the verb / action name, and finally +an optional subject name. For example, given an object 'virHashTable', all +functions should have a name 'virHashTable$VERB' or +'virHashTable$VERB$SUBJECT". e.g. 'virHashTableLookup' or +'virHashTableGetValue'. + + + +*Macro names* + +All macros should have a "VIR" prefix in their name, followed by one or more +uppercase words separated by underscores. The macro argument names should be +in lowercase. Aside from having a "VIR" prefix there are no common practices +for the rest of the macro name. + + + + Code indentation ================ Libvirt's C source code generally adheres to some basic code-formatting diff --git a/docs/hacking.html.in b/docs/hacking.html.in index b1bb149..d904c3a 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -314,6 +314,99 @@ Richard Jones' guide to working with open source projects</a>. </p> + <h2><a name="naming">Naming conventions</a></h2> + + <p> + When reading libvirt code, a number of different naming conventions will + be evident due to various changes in thinking over the course of the + project's lifetime. The conventions documented below should be followed + when creating any entirely new files in libvirt. When working on existing + files, while it is desirable to apply these conventions, keeping a + consistent style with existing code in that particular file is generally + more important. The overall guiding principal is that every file, enum, + struct, function, macro and typedef name must have a 'vir' or 'VIR' prefix. + All local scope variable names are exempt, and global variables are exempt, + unless exported in a header file. + </p> + + <dl> + <dt>File names</dt> + <dd> + <p> + File naming varies depending on the subdirectory. The preferred + style is to have a 'vir' prefix, followed by a name which matches + the name of the functions / objects inside the file. For example, + a file containing an object 'virHashtable' is stored in files + 'virhashtable.c' and 'virhashtable.h'. Sometimes, methods which + would otherwise be declared 'static' need to be exported for use + by a test suite. For this purpose a second header file should be + added with a suffix of 'priv'. e.g. 'virhashtablepriv.h'. Use of + underscores in file names is discouraged when using the 'vir' + prefix style. The 'vir' prefix naming applies to src/util, + src/rpc and tests/ directories. Most other directories do not + follow this convention. + </p> + </dd> + <dt>Enum type & field names</dt> + <dd> + <p> + All enums should have a 'vir' prefix in their typedef name, + and each following word should have its first letter in + uppercase. The enum name should match the typedef name with + a leading underscore. The enum member names should be in all + uppercase, and use an underscore to separate each word. The + enum member name prefix should match the enum typedef name. + </p> + <pre> + typedef enum _virSocketType virSocketType; + enum _virSocketType { + VIR_SOCKET_TYPE_IPV4, + VIR_SOCKET_TYPE_IPV6, + };</pre> + </dd> + <dt>Struct type names</dt> + <dd> + <p> + All structs should have a 'vir' prefix in their typedef name, + and each following word should have its first letter in + uppercase. The struct name should be the same as the typedef + name with a leading underscore. A second typedef should be + given for a pointer to the struct with a 'Ptr' suffix. + </p> + <pre> + typedef struct _virHashTable virHashTable; + typedef virHashTable *virHashTablePtr; + struct _virHashTable { + ... + };</pre> + </dd> + <dt>Function names</dt> + <dd> + <p> + All functions should have a 'vir' prefix in their name, + followed by one or more words with first letter of each + word capitalized. Underscores should not be used in function + names. If the function is operating on an object, then the + function name prefix should match the object typedef name, + otherwise it should match the filename. Following this + comes the verb / action name, and finally an optional + subject name. For example, given an object 'virHashTable', + all functions should have a name 'virHashTable$VERB' or + 'virHashTable$VERB$SUBJECT". e.g. 'virHashTableLookup' + or 'virHashTableGetValue'. + </p> + </dd> + <dt>Macro names</dt> + <dd> + <p> + All macros should have a "VIR" prefix in their name, followed + by one or more uppercase words separated by underscores. The + macro argument names should be in lowercase. Aside from having + a "VIR" prefix there are no common practices for the rest of + the macro name. + </p> + </dd> + </dl> <h2><a name="indent">Code indentation</a></h2> <p> diff --git a/docs/hacking2.xsl b/docs/hacking2.xsl index 3595b7e..7e5ac82 100644 --- a/docs/hacking2.xsl +++ b/docs/hacking2.xsl @@ -114,6 +114,10 @@ from docs/hacking.html.in! <xsl:template match="html:li/html:ul/html:li">-- <xsl:apply-templates/><xsl:value-of select="$newline"/><xsl:value-of select="$newline"/> </xsl:template> +<xsl:template match="html:dl/html:dt">*<xsl:apply-templates/>*<xsl:value-of select="$newline"/><xsl:value-of select="$newline"/> +</xsl:template> +<xsl:template match="html:dl/html:dd"><xsl:apply-templates/><xsl:value-of select="$newline"/><xsl:value-of select="$newline"/> +</xsl:template>  -- 2.9.3

5 7

[libvirt] [PATCH] qemuProcessHandleMonitorEOF: Disable namespace for domain
by Michal Privoznik 10 Mar '17

10 Mar '17

https://bugzilla.redhat.com/show_bug.cgi?id=1430634 If a qemu process has died, we get EOF on its monitor. At this point, since qemu process was the only one running in the namespace kernel has already cleaned the namespace up. Any attempt of ours to enter it has to fail. This really happened in the bug linked above. We've tried to attach a disk to qemu and while we were in the monitor talking to qemu it just died. Therefore our code tried to do some roll back (e.g. deny the device in cgroups again, restore labels, etc.). However, during the roll back (esp. when restoring labels) we still thought that domain has a namespace. So we used secdriver's transactions. This failed as there is no namespace to enter. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_domain.c | 25 +++++++++++++++++++++++++ src/qemu/qemu_domain.h | 3 +++ src/qemu/qemu_process.c | 4 ++++ 3 files changed, 32 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1a42fcf1b..d5833b026 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -201,6 +201,22 @@ qemuDomainEnableNamespace(virDomainObjPtr vm, } +static void +qemuDomainDisableNamespace(virDomainObjPtr vm, + qemuDomainNamespace ns) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (priv->namespaces) { + ignore_value(virBitmapClearBit(priv->namespaces, ns)); + if (virBitmapIsAllClear(priv->namespaces)) { + virBitmapFree(priv->namespaces); + priv->namespaces = NULL; + } + } +} + + void qemuDomainEventQueue(virQEMUDriverPtr driver, virObjectEventPtr event) { @@ -7805,6 +7821,15 @@ qemuDomainCreateNamespace(virQEMUDriverPtr driver, } +void +qemuDomainDestroyNamespace(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{ + if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) + qemuDomainDisableNamespace(vm, QEMU_DOMAIN_NS_MOUNT); +} + + bool qemuDomainNamespaceAvailable(qemuDomainNamespace ns ATTRIBUTE_UNUSED) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 7fa717390..c646828e6 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -816,6 +816,9 @@ int qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg, int qemuDomainCreateNamespace(virQEMUDriverPtr driver, virDomainObjPtr vm); +void qemuDomainDestroyNamespace(virQEMUDriverPtr driver, + virDomainObjPtr vm); + bool qemuDomainNamespaceAvailable(qemuDomainNamespace ns); int qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index fcba7d309..b9c1847bb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -314,6 +314,10 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, */ qemuMonitorUnregister(mon); + /* We don't want any cleanup from EOF handler (or any other + * thread) to enter qemu namespace. */ + qemuDomainDestroyNamespace(driver, vm); + cleanup: virObjectUnlock(vm); } -- 2.11.0

2 2

[libvirt] [PATCH] qemuxml2argvtest: Don't overwrite driver stateDir
by Michal Privoznik 10 Mar '17

10 Mar '17

This is a very historic artefact. Back in the old days of 830ba76c3e when we had macros to add arguments onto qemu command line (!) we thought it was a good idea to let qemu write out the PID file. So we passed -pidfile $stateDir/$domName onto the command line. Thus, in order for tests to work we needed stable stateDir in the qemu driver. Unfortunately, after 16efa11aa696 where stateDir is mkdtemp()-d, this approach lead to a leak of temp dir. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tests/qemuxml2argvtest.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d2d267fce..00b0e25cd 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -581,9 +581,6 @@ mymain(void) if (VIR_STRDUP_QUIET(driver.config->chardevTLSx509certdir, "/etc/pki/libvirt-chardev") < 0) return EXIT_FAILURE; - VIR_FREE(driver.config->stateDir); - if (VIR_STRDUP_QUIET(driver.config->stateDir, "/nowhere") < 0) - return EXIT_FAILURE; VIR_FREE(driver.config->hugetlbfs); if (VIR_ALLOC_N(driver.config->hugetlbfs, 2) < 0) return EXIT_FAILURE; -- 2.11.0

2 1

[libvirt] [PATCH 0/2] qemu: properly handle delayed vCPU hotunplug
by Peter Krempa 10 Mar '17

10 Mar '17

Patch 1 would help in debugging this issue. Patch 2 fixes the bug. Peter Krempa (2): qemu: hotplug: Add debug log when dispatching device removal to existing thread qemu: hotplug: Reset device removal waiting code after vCPU unplug src/qemu/qemu_hotplug.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) -- 2.12.0

3 6

[libvirt] [RFC PATCH 0/2] qemu: Fix ACPI on aarch64
by Andrea Bolognani 09 Mar '17

09 Mar '17

Posted as RFC due to the concerns outlined in patch 1. Andrea Bolognani (2): qemu: Advertise ACPI support for aarch64 guests tests: Initialize basic capabilities properly src/qemu/qemu_capabilities.c | 30 +++++++++++++++++----- .../caps_2.6.0-gicv2.aarch64.xml | 1 + .../caps_2.6.0-gicv3.aarch64.xml | 1 + .../qemuxml2argv-balloon-ccw-deflate.args | 1 - .../qemuxml2argv-console-sclp.args | 1 - .../qemuxml2argv-console-virtio-ccw.args | 1 - .../qemuxml2argv-console-virtio-s390.args | 1 - .../qemuxml2argv-cpu-s390-features.args | 1 - .../qemuxml2argv-cpu-s390-zEC12.args | 1 - .../qemuxml2argv-disk-virtio-ccw-many.args | 1 - .../qemuxml2argv-disk-virtio-ccw.args | 1 - .../qemuxml2argv-disk-virtio-s390.args | 1 - .../qemuxml2argv-disk-virtio-scsi-ccw.args | 1 - tests/qemuxml2argvdata/qemuxml2argv-fs9p-ccw.args | 1 - .../qemuxml2argv-hostdev-scsi-vhost-scsi-ccw.args | 1 - .../qemuxml2argv-hugepages-numa.args | 1 + .../qemuxml2argv-iothreads-disk-virtio-ccw.args | 1 - .../qemuxml2argv-iothreads-virtio-scsi-ccw.args | 1 - .../qemuxml2argv-machine-aeskeywrap-off-cap.args | 1 - .../qemuxml2argv-machine-aeskeywrap-off-caps.args | 1 - .../qemuxml2argv-machine-aeskeywrap-on-cap.args | 1 - .../qemuxml2argv-machine-aeskeywrap-on-caps.args | 1 - .../qemuxml2argv-machine-deakeywrap-off-cap.args | 1 - .../qemuxml2argv-machine-deakeywrap-off-caps.args | 1 - .../qemuxml2argv-machine-deakeywrap-on-cap.args | 1 - .../qemuxml2argv-machine-deakeywrap-on-caps.args | 1 - .../qemuxml2argv-machine-keywrap-none-caps.args | 1 - .../qemuxml2argv-machine-keywrap-none.args | 1 - .../qemuxml2argv-memory-hotplug-ppc64-nonuma.args | 1 - .../qemuxml2argv-net-virtio-ccw.args | 1 - .../qemuxml2argv-net-virtio-s390.args | 1 - tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args | 1 - .../qemuxml2argv-ppce500-serial.args | 1 - .../qemuxml2argv-pseries-basic.args | 1 - .../qemuxml2argv-pseries-cpu-compat.args | 1 - .../qemuxml2argv-pseries-cpu-exact.args | 1 - .../qemuxml2argv-pseries-cpu-le.args | 1 - .../qemuxml2argv-pseries-nvram.args | 1 - .../qemuxml2argv-pseries-panic-missing.args | 1 - .../qemuxml2argv-pseries-panic-no-address.args | 1 - .../qemuxml2argv-pseries-usb-default.args | 1 - .../qemuxml2argv-pseries-usb-kbd.args | 1 - .../qemuxml2argv-pseries-usb-multi.args | 1 - .../qemuxml2argv-pseries-vio-user-assigned.args | 1 - .../qemuxml2argvdata/qemuxml2argv-pseries-vio.args | 1 - .../qemuxml2argv-q35-virt-manager-basic.args | 1 + ...muxml2argv-s390-allow-bogus-usb-controller.args | 1 - .../qemuxml2argv-s390-allow-bogus-usb-none.args | 1 - .../qemuxml2argv-s390-panic-missing.args | 1 - .../qemuxml2argv-s390-panic-no-address.args | 1 - .../qemuxml2argv-virtio-rng-ccw.args | 1 - .../qemuxml2argv-watchdog-diag288.args | 1 - tests/qemuxml2argvtest.c | 16 ++++++++++-- 53 files changed, 41 insertions(+), 56 deletions(-) -- 2.7.4

3 9

[libvirt] [PATCH v2] Revert "conf: move iothread XML validation from qemu_command"
by Pavel Hrdina 09 Mar '17

09 Mar '17

This reverts commit c96bd78e4e71c799dc391566fa9f0652dec55dca. So our code is one big mess and we modify domain definition while building qemu_command line and our hotplug code share only part of the parsing and command line building code. Let's revert that change because to fix it properly would require refactor and move a lot of things. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1430275 --- src/conf/domain_conf.c | 62 ++------------------------------ src/qemu/qemu_command.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 59 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a58f997621..569becb99d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4737,8 +4737,7 @@ virDomainDiskAddressDiskBusCompatibility(virDomainDiskBus bus, static int -virDomainDiskDefValidate(const virDomainDef *def, - const virDomainDiskDef *disk) +virDomainDiskDefValidate(const virDomainDiskDef *disk) { /* Validate LUN configuration */ if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { @@ -4768,24 +4767,6 @@ virDomainDiskDefValidate(const virDomainDef *def, return -1; } - if (disk->iothread > 0) { - if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO || - (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && - disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("IOThreads are only available for virtio pci and " - "virtio ccw disk")); - return -1; - } - - if (!virDomainIOThreadIDFind(def, disk->iothread)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Invalid IOThread id '%u' for disk '%s'"), - disk->iothread, disk->dst); - return -1; - } - } - return 0; } @@ -4837,47 +4818,12 @@ virDomainNetDefValidate(const virDomainNetDef *net) static int -virDomainControllerDefValidate(const virDomainDef *def, - const virDomainControllerDef *cont) -{ - if (cont->iothread > 0) { - if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI || - cont->model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("IOThreads are only supported for virtio-scsi " - "controllers, model is '%s'"), - virDomainControllerModelSCSITypeToString(cont->model)); - return -1; - } - - if (cont->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && - cont->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("IOThreads are only available for virtio pci and " - "virtio ccw controllers")); - return -1; - } - - if (!virDomainIOThreadIDFind(def, cont->iothread)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Invalid IOThread id '%u' for controller '%s'"), - cont->iothread, - virDomainControllerTypeToString(cont->type)); - return -1; - } - } - - return 0; -} - - -static int virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, const virDomainDef *def) { switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: - return virDomainDiskDefValidate(def, dev->data.disk); + return virDomainDiskDefValidate(dev->data.disk); case VIR_DOMAIN_DEVICE_REDIRDEV: return virDomainRedirdevDefValidate(def, dev->data.redirdev); @@ -4885,9 +4831,6 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, case VIR_DOMAIN_DEVICE_NET: return virDomainNetDefValidate(dev->data.net) - case VIR_DOMAIN_DEVICE_CONTROLLER: - return virDomainControllerDefValidate(def, dev->data.controller); - case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: @@ -4895,6 +4838,7 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_HOSTDEV: case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_CONTROLLER: case VIR_DOMAIN_DEVICE_GRAPHICS: case VIR_DOMAIN_DEVICE_HUB: case VIR_DOMAIN_DEVICE_SMARTCARD: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6545a93259..145a0127f2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1871,6 +1871,49 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } +static bool +qemuCheckIOThreads(const virDomainDef *def, + virDomainDiskDefPtr disk) +{ + /* Right "type" of disk" */ + switch ((virDomainDiskBus)disk->bus) { + case VIR_DOMAIN_DISK_BUS_VIRTIO: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads only available for virtio pci and " + "virtio ccw disk")); + return false; + } + break; + + case VIR_DOMAIN_DISK_BUS_IDE: + case VIR_DOMAIN_DISK_BUS_FDC: + case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_USB: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SATA: + case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("IOThreads not available for bus %s target %s"), + virDomainDiskBusTypeToString(disk->bus), disk->dst); + return false; + } + + /* Can we find the disk iothread in the iothreadid list? */ + if (!virDomainIOThreadIDFind(def, disk->iothread)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Disk iothread '%u' not defined in iothreadid"), + disk->iothread); + return false; + } + + return true; +} + + char * qemuBuildDriveDevStr(const virDomainDef *def, virDomainDiskDefPtr disk, @@ -1889,6 +1932,9 @@ qemuBuildDriveDevStr(const virDomainDef *def, if (!qemuCheckCCWS390AddressSupport(def, disk->info, qemuCaps, disk->dst)) goto error; + if (disk->iothread && !qemuCheckIOThreads(def, disk)) + goto error; + switch (disk->bus) { case VIR_DOMAIN_DISK_BUS_IDE: if (disk->info.addr.drive.target != 0) { @@ -2521,6 +2567,52 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def, } +/* qemuCheckSCSIControllerIOThreads: + * @domainDef: Pointer to domain def + * @def: Pointer to controller def + * @qemuCaps: Capabilities + * + * If this controller definition has iothreads set, let's make sure the + * configuration is right before adding to the command line + * + * Returns true if either supported or there are no iothreads for controller; + * otherwise, returns false if configuration is not quite right. + */ +static bool +qemuCheckSCSIControllerIOThreads(const virDomainDef *domainDef, + virDomainControllerDefPtr def) +{ + if (!def->iothread) + return true; + + if (def->model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("IOThreads only supported for virtio-scsi " + "controllers model is '%s'"), + virDomainControllerModelSCSITypeToString(def->model)); + return false; + } + + if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads only available for virtio pci and " + "virtio ccw controllers")); + return false; + } + + /* Can we find the controller iothread in the iothreadid list? */ + if (!virDomainIOThreadIDFind(domainDef, def->iothread)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("controller iothread '%u' not defined in iothreadid"), + def->iothread); + return false; + } + + return true; +} + + char * qemuBuildControllerDevStr(const virDomainDef *domainDef, virDomainControllerDefPtr def, @@ -2571,6 +2663,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { virBufferAddLit(&buf, "virtio-scsi-ccw"); if (def->iothread) { + if (!qemuCheckSCSIControllerIOThreads(domainDef, def)) + goto error; virBufferAsprintf(&buf, ",iothread=iothread%u", def->iothread); } @@ -2583,6 +2677,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, } else { virBufferAddLit(&buf, "virtio-scsi-pci"); if (def->iothread) { + if (!qemuCheckSCSIControllerIOThreads(domainDef, def)) + goto error; virBufferAsprintf(&buf, ",iothread=iothread%u", def->iothread); } -- 2.12.0

2 1

[libvirt] Revisiting qemu monitor timeout on VM start
by Jim Fehlig 09 Mar '17

09 Mar '17

Hi All, Encountering a qemu monitor timeout when starting a VM has been discussed here before, e.g. https://www.redhat.com/archives/libvir-list/2014-January/msg00060.html https://www.redhat.com/archives/libvir-list/2014-January/msg00408.html Recently I've received reports of the same when starting large memory VMs backed by 1G huge pages. In one of the reports, Matt timed how long it takes to allocate 402GB worth of hugetlbfs pages (these are 1G pages, but the time is similar for 2M): real 105.47 user 0.05 sys 105.42 The time is spent entirely in the kernel zero'ing pages and as you can see it exceeds the 30 second monitor timeout in libvirt. Do folks have any suggestions on how to avoid the timeout? Obviously one solution is to introduce a knob in qemu.conf to control the timeout, as was proposed in the above threads. Another solution that came to mind is changing qemu to open the monitor earlier, making it available while the kernel is off scrubbing pages. I'm not familiar enough with qemu code to know if such a change is possible, but given the amount of initialization done in main() prior to calling mon_init_func(), my confidence in this idea is low. Perhaps someone more familiar with qemu initialization can comment on that. Thanks in advance for comments on these ideas or alternate proposals! Regards, Jim

3 2

[libvirt] [PATCH] configure: disable scsi stroage driver on non-Linux
by Roman Bogorodskiy 09 Mar '17

09 Mar '17

Even though scsi storage driver builds fine on non-Linux, it will not work properly because it relies on Linux procfs, so disable that in configure if we're not building for Linux. --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index e61ab7ba6..cf50422d5 100644 --- a/configure.ac +++ b/configure.ac @@ -192,6 +192,7 @@ if test $with_linux = no; then with_lxc=no fi with_dtrace=no + with_storage_scsi=no fi if test $with_freebsd = yes; then -- 2.11.0

2 2