March 2017 - Devel - libvirt List Archives

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

11 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 11 Mar '17

11 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 10 Mar '17

10 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 10 Mar '17

10 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 10 Mar '17

10 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 10 Mar '17

10 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

[libvirt] [PATCH v2 00/14] Introduce NVDIMM support
by Michal Privoznik 09 Mar '17

09 Mar '17

v2 of: https://www.redhat.com/archives/libvir-list/2017-February/msg01229.html How to test this feature? $ > /tmp/nvdimm $ truncate --size 512M /tmp/nvdimm $ virsh edit $mydom <memory model='nvdimm' access='shared'> <source> <path>/tmp/nvdimm</path> </source> <target> <size unit='MiB'>512</size> <node>0</node> <label> <size unit='KiB'>128</size> </label> </target> <address type='dimm' slot='0'/> </memory> $ virsh start $mydom $ ssh $mydom "echo \"Hello world\" > /dev/pmem0" $ hexdump -C /tmp/nvdimm 00000000 48 65 6c 6c 6f 20 77 6f 72 6c 64 0a 00 00 00 00 |Hello world.....| 00000010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 20000000 Michal Privoznik (14): qemuBuildMemoryBackendStr: Reorder args and update comment Introduce NVDIMM memory model qemu: Introduce QEMU_CAPS_DEVICE_NVDIMM qemu: Implement NVDIMM conf: Introduce @access to <memory/> qemu: Implement @access for <memory/> banks qemu: Introduce label-size for NVDIMMs security_dac: Label host side of NVDIMM security_selinux: Label host side of NVDIMM security: Introduce internal APIs for memdev labelling secdrivers: Implement memdev relabel APIs qemu_hotplug: Relabel memdev qemu: Allow nvdimm in devices CGroups qemu: Namespaces for NVDIMM docs/formatdomain.html.in | 79 ++++++++--- docs/schemas/domaincommon.rng | 47 +++++-- src/conf/domain_conf.c | 131 +++++++++++++---- src/conf/domain_conf.h | 5 + src/libvirt_private.syms | 2 + src/qemu/qemu_alias.c | 10 +- src/qemu/qemu_capabilities.c | 9 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_cgroup.c | 49 +++++++ src/qemu/qemu_cgroup.h | 4 + src/qemu/qemu_command.c | 155 +++++++++++++++------ src/qemu/qemu_command.h | 16 ++- src/qemu/qemu_domain.c | 105 +++++++++++++- src/qemu/qemu_domain.h | 8 ++ src/qemu/qemu_hotplug.c | 42 +++++- src/qemu/qemu_security.c | 56 ++++++++ src/qemu/qemu_security.h | 8 ++ src/security/security_dac.c | 76 ++++++++++ src/security/security_driver.h | 9 ++ src/security/security_manager.c | 56 ++++++++ src/security/security_manager.h | 7 + src/security/security_nop.c | 19 +++ src/security/security_selinux.c | 69 +++++++++ src/security/security_stack.c | 38 +++++ .../qemuxml2argv-memory-hotplug-nvdimm-access.args | 26 ++++ .../qemuxml2argv-memory-hotplug-nvdimm-access.xml | 56 ++++++++ .../qemuxml2argv-memory-hotplug-nvdimm-label.args | 26 ++++ .../qemuxml2argv-memory-hotplug-nvdimm-label.xml | 59 ++++++++ .../qemuxml2argv-memory-hotplug-nvdimm.args | 26 ++++ .../qemuxml2argv-memory-hotplug-nvdimm.xml | 56 ++++++++ tests/qemuxml2argvtest.c | 8 +- ...qemuxml2xmlout-memory-hotplug-nvdimm-access.xml | 1 + .../qemuxml2xmlout-memory-hotplug-nvdimm-label.xml | 1 + .../qemuxml2xmlout-memory-hotplug-nvdimm.xml | 1 + tests/qemuxml2xmltest.c | 3 + 35 files changed, 1144 insertions(+), 120 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-label.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-access.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm-label.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-memory-hotplug-nvdimm.xml -- 2.11.0

2 37

[libvirt] [PATCH] domain_conf: move iothread check for address type back to qemu_command
by Pavel Hrdina 09 Mar '17

09 Mar '17

This partially reverts commit c96bd78e4e. If an API virDomainAttachDevice(Flags) is used the device address is assigned after the validation and the address type may not be set. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1430275 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/conf/domain_conf.c | 18 ++++-------------- src/qemu/qemu_command.c | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 97d42fe993..747706e673 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4769,12 +4769,10 @@ virDomainDiskDefValidate(const virDomainDef *def, } 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")); + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("IOThreads are not available for bus '%s', disk '%s'"), + virDomainDiskBusTypeToString(disk->bus), disk->dst); return -1; } @@ -4850,14 +4848,6 @@ virDomainControllerDefValidate(const virDomainDef *def, 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'"), diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6545a93259..b49242a7df 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1871,6 +1871,21 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } +static int +qemuBuildCheckIOThreadAddress(virDomainDeviceInfo info) +{ + if (info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && + info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads are only available for virtio pci and " + "virtio ccw")); + return -1; + } + + return 0; +} + + char * qemuBuildDriveDevStr(const virDomainDef *def, virDomainDiskDefPtr disk, @@ -1889,6 +1904,9 @@ qemuBuildDriveDevStr(const virDomainDef *def, if (!qemuCheckCCWS390AddressSupport(def, disk->info, qemuCaps, disk->dst)) goto error; + if (disk->iothread > 0 && qemuBuildCheckIOThreadAddress(disk->info) < 0) + goto error; + switch (disk->bus) { case VIR_DOMAIN_DISK_BUS_IDE: if (disk->info.addr.drive.target != 0) { @@ -2571,6 +2589,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { virBufferAddLit(&buf, "virtio-scsi-ccw"); if (def->iothread) { + if (qemuBuildCheckIOThreadAddress(def->info) < 0) + goto error; virBufferAsprintf(&buf, ",iothread=iothread%u", def->iothread); } @@ -2583,6 +2603,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, } else { virBufferAddLit(&buf, "virtio-scsi-pci"); if (def->iothread) { + if (qemuBuildCheckIOThreadAddress(def->info) < 0) + goto error; virBufferAsprintf(&buf, ",iothread=iothread%u", def->iothread); } -- 2.12.0

2 1