- Devel - libvirt List Archives

[libvirt] [REPOST PATCH 0/6] Have 'buildVol' callers to clean up after themselves
by John Ferlan 04 Nov '15

04 Nov '15

Since some time and changes have occurred since the original posting, I figured it would be just easier to updated and repost the remainder of the series: http://www.redhat.com/archives/libvir-list/2015-October/msg00312.html Where the patches 1-3 are v2's of the original patches 8 & 9: http://www.redhat.com/archives/libvir-list/2015-October/msg00515.html http://www.redhat.com/archives/libvir-list/2015-October/msg00516.html http://www.redhat.com/archives/libvir-list/2015-October/msg00518.html Note that patches 4-6 have been ACK'd already as patches 10-12, it's just that without the previous patches it doesn't make much sense to push them alone. John Ferlan (6): storage: Refactor and fix virStorageBackendCreateExecCommand storage: Cleanup failures virStorageBackendCreateExecCommand storage: Cleanup failures in virStorageBackendCreateRaw storage: Pull volume removal from pool in storageVolDeleteInternal Revert "storage: Prior to creating a volume, refresh the pool" storage: On 'buildVol' failure don't delete the volume src/storage/storage_backend.c | 40 ++++++++++++++++++++++++++------------- src/storage/storage_backend.h | 10 ++++++++++ src/storage/storage_driver.c | 44 +++++++++++++++++++++---------------------- 3 files changed, 59 insertions(+), 35 deletions(-) -- 2.1.0

2 17

[libvirt] [PATCH] Revert "utils: Remove the logging of errors from virNetDevSendEthtoolIoctl"
by Daniel P. Berrange 03 Nov '15

03 Nov '15

This reverts commit 6f2a0198e913c91a2ef8b99db79b7d3cc5396957. This commit removed error reporting from virNetDevSendEthtoolIoctl pushing responsibility onto the callers. This is wrong, however, since virNetDevSendEthtoolIoctl calls virNetDevSetupControl which can still report errors. So as a result virNetDevSendEthtoolIoctl may or may not report errors depending on which bit of it fails, and as a result callers now overwrite some errors. It also introduced a regression causing unprivileged libvirtd to spew error messages to the console due to inability to query the NIC features, an error which was previously ignored. virNetDevSetupControlFull:148 : Cannot open network interface control socket: Operation not permitted virNetDevFeatureAvailable:3062 : Cannot get device wlp3s0 flags: Operation not permitted virNetDevSetupControlFull:148 : Cannot open network interface control socket: Operation not permitted virNetDevFeatureAvailable:3062 : Cannot get device wlp3s0 flags: Operation not permitted virNetDevSetupControlFull:148 : Cannot open network interface control socket: Operation not permitted virNetDevFeatureAvailable:3062 : Cannot get device wlp3s0 flags: Operation not permitted virNetDevSetupControlFull:148 : Cannot open network interface control socket: Operation not permitted virNetDevFeatureAvailable:3062 : Cannot get device wlp3s0 flags: Operation not permitted Looking back at the original posting I see no explanation of why thsi refactoring was needed, so reverting the clearly broken error reporting logic looks like the best option. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/util/virnetdev.c | 56 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 9789e93..657df3a 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -3145,15 +3145,39 @@ static int virNetDevSendEthtoolIoctl(const char *ifname, void *cmd) { int ret = -1; - int fd; - struct ifreq ifr; + int sock = -1; + virIfreq ifr; - if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) - return ret; + sock = socket(AF_LOCAL, SOCK_DGRAM, 0); + if (sock < 0) { + virReportSystemError(errno, "%s", _("Cannot open control socket")); + goto cleanup; + } + + memset(&ifr, 0, sizeof(ifr)); + strcpy(ifr.ifr_name, ifname); ifr.ifr_data = cmd; - ret = ioctl(fd, SIOCETHTOOL, &ifr); + ret = ioctl(sock, SIOCETHTOOL, &ifr); + if (ret != 0) { + switch (errno) { + case EPERM: + VIR_DEBUG("ethtool ioctl: permission denied"); + break; + case EINVAL: + VIR_DEBUG("ethtool ioctl: invalid request"); + break; + case EOPNOTSUPP: + VIR_DEBUG("ethtool ioctl: request not supported"); + break; + default: + virReportSystemError(errno, "%s", _("ethtool ioctl error")); + goto cleanup; + } + } - VIR_FORCE_CLOSE(fd); + cleanup: + if (sock) + VIR_FORCE_CLOSE(sock); return ret; } @@ -3170,12 +3194,12 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd) static int virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd) { + int ret = -1; + cmd = (void*)cmd; - if (virNetDevSendEthtoolIoctl(ifname, cmd) < 0) { - virReportSystemError(errno, _("Cannot get device %s flags"), ifname); - return -1; - } - return cmd->data > 0 ? 1 : 0; + if (!virNetDevSendEthtoolIoctl(ifname, cmd)) + ret = cmd->data > 0 ? 1 : 0; + return ret; } @@ -3192,12 +3216,12 @@ virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd) static int virNetDevGFeatureAvailable(const char *ifname, struct ethtool_gfeatures *cmd) { + int ret = -1; + cmd = (void*)cmd; - if (virNetDevSendEthtoolIoctl(ifname, cmd) < 0) { - virReportSystemError(errno, _("Cannot get device %s generic features"), ifname); - return -1; - } - return FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active); + if (!virNetDevSendEthtoolIoctl(ifname, cmd)) + ret = FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active); + return ret; } # endif -- 2.5.0

2 4

[libvirt] [PATCH] virkeymaps.h: Fix VPATH build with --disable-dependency-tracking
by Michal Privoznik 03 Nov '15

03 Nov '15

Funny things happen when you try to do a VPATH build and pass --disable-dependency-tracking argument to the configure script. Not only the directory structure is not created at the end of configure phase, but also contradictory to others, virkeymaps.h does not depend on src/utils dirstamp. I don't know how to force automake to generate that rule, but this is the diff that makes things work for me again. Yes, as you can see it more or less reverts a9fe620372144db3e432bd7506cb909cc3179ef8. More than less. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Steps to reproduce: 1) mkdir /tmp/test_libvirt && cd $_ 2) $OLDPWD/configure --disable-dependency-tracking 3) make src/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index 99b4993..f98912f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -181,7 +181,7 @@ MAINTAINERCLEANFILES += util/virkeymaps.h util/virkeymaps.h: $(srcdir)/util/keymaps.csv \ $(srcdir)/util/virkeycode-mapgen.py $(AM_V_GEN)$(PYTHON) $(srcdir)/util/virkeycode-mapgen.py \ - <$(srcdir)/util/keymaps.csv >util/virkeymaps.h + <$(srcdir)/util/keymaps.csv >$(srcdir)/util/virkeymaps.h # Internal generic driver infrastructure NODE_INFO_SOURCES = nodeinfo.h nodeinfo.c nodeinfopriv.h -- 2.4.10

3 5

[libvirt] [PATCH] bhyve: add bootrom support
by Roman Bogorodskiy 03 Nov '15

03 Nov '15

Recently, bhyve got UEFI support that eliminates necessity of having two-step process of starting a VM, i.e. loading OS using loader like bhyveloader or grub and then calling bhyve itself. >From user perspective usage looks the following: if a domain XML contains the '<bootloader>' element and it's value is a path to non-executable file, then it's treated as a path to bootrom file and external loader is not execute. All the existing behavior remains the same, e.g. if bootloader is not specified, bhyveloader is used. If loader is specified and it points to an executable file, it's executed before running bhyve. Implementation consists of two major parts: * Add probing for capability by checking if the current bhyve binary accepts '-l bootrom' argument meaning that UEFI is supported * virBhyveProcessBuildBhyveCmd is extended with 'external_loader' argument. If it decides (based on the schema described above) that external loader is not needed, it sets it to false. All the related code changed accordingly to act based on value of external_loader. --- src/bhyve/bhyve_capabilities.c | 11 ++++ src/bhyve/bhyve_capabilities.h | 1 + src/bhyve/bhyve_command.c | 20 +++++++- src/bhyve/bhyve_command.h | 1 + src/bhyve/bhyve_driver.c | 15 +++--- src/bhyve/bhyve_process.c | 58 ++++++++++++---------- .../bhyvexml2argvdata/bhyvexml2argv-bhyveload.args | 3 ++ .../bhyvexml2argv-bhyveload.ldargs | 1 + .../bhyvexml2argvdata/bhyvexml2argv-bhyveload.xml | 24 +++++++++ tests/bhyvexml2argvdata/bhyvexml2argv-bootrom.args | 4 ++ tests/bhyvexml2argvdata/bhyvexml2argv-bootrom.xml | 23 +++++++++ tests/bhyvexml2argvmock.c | 13 +++++ tests/bhyvexml2argvtest.c | 39 ++++++++------- 13 files changed, 162 insertions(+), 51 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bootrom.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bootrom.xml diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index d2970a2..fbdd63c 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -166,6 +166,17 @@ virBhyveProbeCaps(unsigned int *caps) if (strstr(help, "-u:") != NULL) *caps |= BHYVE_CAP_RTC_UTC; + virCommandFree(cmd); + + cmd = virCommandNew(binary); + virCommandAddArgList(cmd, "-l", "bootrom", NULL); + if (virCommandRun(cmd, &exit) < 0) { + ret = -1; + goto out; + } + if (exit == 1) + *caps |= BHYVE_CAP_BOOTROM; + out: VIR_FREE(help); virCommandFree(cmd); diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index 0eb22a4..25dfb2c 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -33,6 +33,7 @@ typedef enum { typedef enum { BHYVE_CAP_RTC_UTC = 1, + BHYVE_CAP_BOOTROM = 2, } virBhyveCapsFlags; int virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps); diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 6576029..10d504e 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -216,7 +216,9 @@ bhyveBuildDiskArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, virCommandPtr virBhyveProcessBuildBhyveCmd(virConnectPtr conn, - virDomainDefPtr def, bool dryRun) + virDomainDefPtr def, + bool *external_loader, + bool dryRun) { /* * /usr/sbin/bhyve -c 2 -m 256 -AI -H -P \ @@ -230,6 +232,22 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, virCommandPtr cmd = virCommandNew(BHYVE); + /* Check what loader to use */ + if ((def->os.bootloader != NULL) && + (!virFileIsExecutable(def->os.bootloader))) { + *external_loader = false; + if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_BOOTROM) == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Installed bhyve binary does not support " + "bootrom")); + goto error; + } + virCommandAddArg(cmd, "-l"); + virCommandAddArgFormat(cmd, "bootrom,%s", def->os.bootloader); + } else { + *external_loader = true; + } + /* CPUs */ virCommandAddArg(cmd, "-c"); virCommandAddArgFormat(cmd, "%d", def->vcpus); diff --git a/src/bhyve/bhyve_command.h b/src/bhyve/bhyve_command.h index 22a959d..20a3bf0 100644 --- a/src/bhyve/bhyve_command.h +++ b/src/bhyve/bhyve_command.h @@ -32,6 +32,7 @@ virCommandPtr virBhyveProcessBuildBhyveCmd(virConnectPtr conn, virDomainDefPtr def, + bool *external_loader, bool dryRun); virCommandPtr diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 4840a0e..7290251 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -698,6 +698,7 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn, virDomainDefPtr def = NULL; virCommandPtr cmd = NULL, loadcmd = NULL; virCapsPtr caps = NULL; + bool external_loader = NULL; char *ret = NULL; virCheckFlags(0, NULL); @@ -721,15 +722,17 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn, if (bhyveDomainAssignAddresses(def, NULL) < 0) goto cleanup; - if (!(loadcmd = virBhyveProcessBuildLoadCmd(conn, def, "<device.map>", - NULL))) + if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, def, &external_loader, true))) goto cleanup; - if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, def, true))) - goto cleanup; + if (external_loader) { + if (!(loadcmd = virBhyveProcessBuildLoadCmd(conn, def, "<device.map>", + NULL))) + goto cleanup; - virBufferAdd(&buf, virCommandToString(loadcmd), -1); - virBufferAddChar(&buf, '\n'); + virBufferAdd(&buf, virCommandToString(loadcmd), -1); + virBufferAddChar(&buf, '\n'); + } virBufferAdd(&buf, virCommandToString(cmd), -1); if (virBufferCheckError(&buf) < 0) diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index 284641a..d62cccd 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -114,6 +114,7 @@ virBhyveProcessStart(virConnectPtr conn, virCommandPtr load_cmd = NULL; bhyveConnPtr privconn = conn->privateData; int ret = -1, rc; + bool external_loader = NULL; if (virAsprintf(&logfile, "%s/%s.log", BHYVE_LOG_DIR, vm->def->name) < 0) @@ -150,6 +151,7 @@ virBhyveProcessStart(virConnectPtr conn, /* Call bhyve to start the VM */ if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, vm->def, + &external_loader, false))) goto cleanup; @@ -159,38 +161,40 @@ virBhyveProcessStart(virConnectPtr conn, virCommandSetPidFile(cmd, privconn->pidfile); virCommandDaemonize(cmd); - /* Now bhyve command is constructed, meaning the - * domain is ready to be started, so we can build - * and execute bhyveload command */ - rc = virBhyveFormatDevMapFile(vm->def->name, &devmap_file); - if (rc < 0) - goto cleanup; - - if (!(load_cmd = virBhyveProcessBuildLoadCmd(conn, vm->def, devmap_file, - &devicemap))) - goto cleanup; - virCommandSetOutputFD(load_cmd, &logfd); - virCommandSetErrorFD(load_cmd, &logfd); + if (external_loader) { + /* Now bhyve command is constructed, meaning the + * domain is ready to be started, so we can build + * and execute bhyveload command */ + rc = virBhyveFormatDevMapFile(vm->def->name, &devmap_file); + if (rc < 0) + goto cleanup; - if (devicemap != NULL) { - rc = virFileWriteStr(devmap_file, devicemap, 0644); - if (rc) { - virReportSystemError(errno, - _("Cannot write device.map '%s'"), - devmap_file); + if (!(load_cmd = virBhyveProcessBuildLoadCmd(conn, vm->def, devmap_file, + &devicemap))) goto cleanup; + virCommandSetOutputFD(load_cmd, &logfd); + virCommandSetErrorFD(load_cmd, &logfd); + + if (devicemap != NULL) { + rc = virFileWriteStr(devmap_file, devicemap, 0644); + if (rc) { + virReportSystemError(errno, + _("Cannot write device.map '%s'"), + devmap_file); + goto cleanup; + } } - } - /* Log generated command line */ - virCommandWriteArgLog(load_cmd, logfd); - if ((pos = lseek(logfd, 0, SEEK_END)) < 0) - VIR_WARN("Unable to seek to end of logfile: %s", - virStrerror(errno, ebuf, sizeof(ebuf))); + /* Log generated command line */ + virCommandWriteArgLog(load_cmd, logfd); + if ((pos = lseek(logfd, 0, SEEK_END)) < 0) + VIR_WARN("Unable to seek to end of logfile: %s", + virStrerror(errno, ebuf, sizeof(ebuf))); - VIR_DEBUG("Loading domain '%s'", vm->def->name); - if (virCommandRun(load_cmd, NULL) < 0) - goto cleanup; + VIR_DEBUG("Loading domain '%s'", vm->def->name); + if (virCommandRun(load_cmd, NULL) < 0) + goto cleanup; + } /* Now we can start the domain */ VIR_DEBUG("Starting domain '%s'", vm->def->name); diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload.args b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload.args new file mode 100644 index 0000000..118735e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload.args @@ -0,0 +1,3 @@ +/usr/sbin/bhyve -c 1 -m 214 -u -H -P -s 0:0,hostbridge \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 2:0,ahci-hd,/tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload.ldargs new file mode 100644 index 0000000..56f7fb3 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload.ldargs @@ -0,0 +1 @@ +/usr/sbin/bhyveload -foo diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload.xml new file mode 100644 index 0000000..9dac10c --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload.xml @@ -0,0 +1,24 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <bootloader>/usr/sbin/bhyveload</bootloader> + <bootloader_args>-foo</bootloader_args> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bootrom.args b/tests/bhyvexml2argvdata/bhyvexml2argv-bootrom.args new file mode 100644 index 0000000..d46ffab --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bootrom.args @@ -0,0 +1,4 @@ +/usr/sbin/bhyve -l bootrom,/path/to/rom.fd \ +-c 1 -m 214 -u -H -P -s 0:0,hostbridge \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 2:0,ahci-hd,/tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bootrom.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-bootrom.xml new file mode 100644 index 0000000..2da80c5 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bootrom.xml @@ -0,0 +1,23 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <bootloader>/path/to/rom.fd</bootloader> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvmock.c b/tests/bhyvexml2argvmock.c index 41058ca..797dc20 100644 --- a/tests/bhyvexml2argvmock.c +++ b/tests/bhyvexml2argvmock.c @@ -1,5 +1,6 @@ #include <config.h> +#include "virfile.h" #include "virstring.h" #include "virnetdev.h" #include "virnetdevtap.h" @@ -48,3 +49,15 @@ int virNetDevSetOnline(const char *ifname ATTRIBUTE_UNUSED, { return 0; } + +bool virFileIsExecutable(const char *file) +{ + if (STREQ(file, "/fizz_buzz_bazz")) + return true; + else if (STREQ(file, "/usr/local/sbin/grub-bhyve")) + return true; + else if (STREQ(file, "/usr/sbin/bhyveload")) + return true; + + return false; +} diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index 3e57a78..2ff1ae3 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -23,6 +23,7 @@ static int testCompareXMLToArgvFiles(const char *xml, virDomainDefPtr vmdef = NULL; virCommandPtr cmd = NULL, ldcmd = NULL; virConnectPtr conn; + bool external_loader = NULL; int ret = -1; if (!(conn = virGetConnect())) @@ -34,33 +35,35 @@ static int testCompareXMLToArgvFiles(const char *xml, conn->privateData = &driver; - if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, vmdef, false))) + if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, vmdef, &external_loader, false))) goto out; if (!(actualargv = virCommandToString(cmd))) goto out; - if (!(ldcmd = virBhyveProcessBuildLoadCmd(conn, vmdef, "<device.map>", - &actualdm))) - goto out; - - if (actualdm != NULL) - virTrimSpaces(actualdm, NULL); - - if (!(actualld = virCommandToString(ldcmd))) - goto out; + if (external_loader) { + if (!(ldcmd = virBhyveProcessBuildLoadCmd(conn, vmdef, "<device.map>", + &actualdm))) + goto out; - if (virtTestCompareToFile(actualargv, cmdline) < 0) - goto out; + if (actualdm != NULL) + virTrimSpaces(actualdm, NULL); - if (virtTestCompareToFile(actualld, ldcmdline) < 0) - goto out; + if (!(actualld = virCommandToString(ldcmd))) + goto out; - if (virFileExists(dmcmdline) || actualdm) { - if (virtTestCompareToFile(actualdm, dmcmdline) < 0) + if (virtTestCompareToFile(actualld, ldcmdline) < 0) goto out; + + if (virFileExists(dmcmdline) || actualdm) { + if (virtTestCompareToFile(actualdm, dmcmdline) < 0) + goto out; + } } + if (virtTestCompareToFile(actualargv, cmdline) < 0) + goto out; + ret = 0; out: @@ -118,7 +121,7 @@ mymain(void) } while (0) driver.grubcaps = BHYVE_GRUB_CAP_CONSDEV; - driver.bhyvecaps = BHYVE_CAP_RTC_UTC; + driver.bhyvecaps = BHYVE_CAP_RTC_UTC | BHYVE_CAP_BOOTROM; DO_TEST("base"); DO_TEST("acpiapic"); @@ -130,8 +133,10 @@ mymain(void) DO_TEST("grub-defaults"); DO_TEST("grub-bootorder"); DO_TEST("grub-bootorder2"); + DO_TEST("bhyveload"); DO_TEST("bhyveload-explicitargs"); DO_TEST("custom-loader"); + DO_TEST("bootrom"); DO_TEST("disk-cdrom-grub"); DO_TEST("serial-grub"); DO_TEST("localtime"); -- 2.6.1

2 3

[libvirt] Ten years of libvirt
by Michal Privoznik 03 Nov '15

03 Nov '15

Dear list, join me on this big day in congratulations to libvirt that has just turned 10 and is starting new decade of its life. At the same time big thanks to all who contributed in any way. Cheers! Michal

6 6

Re: [libvirt] [PATCH 7/8] Postpone reprobing till all the devices in iommu group are unbound from vfio
by Andrea Bolognani 02 Nov '15

02 Nov '15

On Mon, 2015-11-02 at 10:06 +0530, Shivaprasad bhat wrote: > > > +{ > > > + int ret = 0; > > > + virPCIDevicePtr pci = NULL; > > > + > > > + if (!(pci = virPCIDeviceNew(devAddr->domain, devAddr->bus, > > > + devAddr->slot, devAddr->function))) > > > + goto cleanup; > > > + > > > + if (STREQ_NULLABLE(pci->stubDriver, "vfio-pci")) > > > + ret = -1; > > > > As mentioned in the comment for Patch 1, I think this is > > fairly obscure: if I had not read throught the whole > > series, I'd assume this checks whether the device is > > configured to use vfio-pci as stub driver, not whether > > it is currently bound to it, and it would not be > > immediately clear to me how this check fits in a function > > called virPCIDeviceBoundToVFIODriver(). > > > > I think it would be much cleaner to query the driver > > explicitly using virPCIDeviceGetDriverPathAndName() and > > remove that call from virPCIDeviceNew(). > > > > The PCIDeviceNew does exactly the same of going through > the sysfs and populating it. Its only code which is as though > the stubDriver scan is happening inside than explicitly outside. > > The Iterator today takes virPCIDeviceAddressPtr and > virPCIDeviceGetDriverPathAndName for which we anyway need to > call virPCIDevNew. I am just avoiding one extra function call by > moving it inside virPCIDevNew The problem I have with this approach is that, as far as I understand, before your patch dev->stubDriver is a configuration item: you set it to whatever's appropriate, and the value is later inspected in virPCIDeviceBindToStub()[1] to decide what stub driver the device should be bound to. After your patch, depending on the context, it could be either a configuration item, as before, or state, eg. the current stub driver the device is bound to, with no clear way to tell one situation from the other. I think using a single attribute to store related but different information should be avoided, as it has the potential to make the code much harder to understand, especially later on for someone who's not been involved in the implementation. Moreover you only need that value for comparison purposes in a couple of places, and by moving the code to virPCIDeviceNew() you make it so the value is loaded (by performing filesystem operations) every single time a virDevice is created, whether the value is actually used or not. Cheers. [1] Rather virPCIDeviceDetach() in reality - I have a pending patch that moves it to virPCIDeviceBindToStub() where it IMHO belongs -- Andrea Bolognani Software Engineer - Virtualization Team

1 0

[libvirt] [PATCH v6 00/13] qemu: Add quorum support to libvirt
by Matthias Gatto 02 Nov '15

02 Nov '15

The purpose of these patches is to introduce quorum for libvirt I've try to follow this proposal: http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html This feature ask for 6 task: 1) Allow a _virStorageSource to contain more than one backing store. Because all the actual libvirt code use the backingStore field as a pointer and we needs want to change that, I've decide to encapsulate the backingStore field to simplifie the array manipulation. 2) Add the missing field a quorum need in _virStorageSource and the VIR_STORAGE_TYPE_QUORUM and VIR_STORAGE_FILE_QUORUM in their respectives enums. 3) Parse and format the xml Because a quorum allows to have more than one backing store at the same level we need to change virDomainDiskDefFormat and virDomainDiskDefParseXML to call virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse in a loop. virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse can call themself recursively in a loop because a quorum can contain another quorum 4) Build qemu string As for the xml, we have to call the function which create quorum recursively. But this task have the problem explained here: http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html The _virStorageSource missing some informations that can be passed to a child, and therefore this version of quorum is incomplet. 5) Allow to hotplug/change a disk in a quorum This part is not present in these patches because for this task we have to use blockdev-add, and currently libvirt use device_add for hotpluging that doesn't allow to hotplug quorum childs. There is 3 way to handle this problem: 1) create a virDomainBlockDevAdd function in libvirt witch call blockdev-add. 2) use blockdev-add instead of device_add in qemuMonitorJSONAddDevice 3) write a hack which uses blockdev-add only when attaching a quorum child V2: -Rebase on master -Add Documentation V3: -Transforme the backingStore field in virStorageSource into an array of pointer instead of a pointer -Modify virStorageSourceSetBackingStore to allow it to expand the backingStore size. V4: -Rebase on master V5: -Rebase on master -patch 1-4/9: use patchs from John Ferlan -patch 4/9: check return of virStorageSourceSetBackingStore -patch 5/9: report type of error on virStorageSourceSetBackingStore v6 note: First at all, I'm sorry for the time between v5 and v6, I had other projects to work on and was unable to work at full time on libvirt, moreover I've try at first to support all snapshot and block jobs operations for quorum, but encounter a lot a problems. At the end I had a versions which was only handling some few operations, so I've block all unsupported operations for quorum, I plan to continue working on the quorum unsupported operations, and send it when it will be ready, but in the meantime I hope to upstream this serie of patch which should provide a very basic(but stable) quorum suport. v6 modifications: -Rebase on master -Remove node-name patch -patch 06/13: Add virStorageSourceIsRAID -patch 10/13: Add virDomainDefHasRAID -patch 11-13/13: Block all unsupported operations Matthias Gatto (13): virstoragefile: Add virStorageSourceGetBackingStore virstoragefile: Always use virStorageSourceGetBackingStore to get backing store virstoragefile: Add virStorageSourceSetBackingStore virstoragefile: Always use virStorageSourceSetBackingStore to set backing store virstoragefile: change backingStore to backingStores. virstoragefile: Add virStorageSourceIsRAID virstoragefile: Add quorum in virstoragefile domain_conf: Read and Write quorum config qemu: Add quorum support in qemuBuildDriveDevStr domain: add virDomainDefHasRAID qemu: Block snapshot operation with RAID qemu: qemuDomainGetBlockInfo quorum support qemu: qemuDiskPathToAlias quorum support docs/formatdomain.html.in | 23 +++- docs/schemas/domaincommon.rng | 21 +++- docs/schemas/storagecommon.rng | 1 + docs/schemas/storagevol.rng | 1 + src/conf/domain_conf.c | 196 +++++++++++++++++++++++++--------- src/conf/domain_conf.h | 1 + src/conf/storage_conf.c | 23 ++-- src/libvirt_private.syms | 4 + src/qemu/qemu_cgroup.c | 4 +- src/qemu/qemu_command.c | 94 ++++++++++++++++ src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 50 ++++++--- src/qemu/qemu_migration.c | 1 + src/qemu/qemu_monitor_json.c | 4 +- src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 +- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 22 ++-- src/storage/storage_backend_fs.c | 38 ++++--- src/storage/storage_backend_gluster.c | 11 +- src/storage/storage_backend_logical.c | 15 ++- src/storage/storage_driver.c | 3 +- src/util/virstoragefile.c | 118 +++++++++++++++++--- src/util/virstoragefile.h | 15 ++- tests/virstoragetest.c | 18 ++-- 25 files changed, 523 insertions(+), 150 deletions(-) -- 2.6.1

2 23

[libvirt] [PATCH v3 0/9] Few VFIO related fixes
by Shivaprasad G Bhat 02 Nov '15

02 Nov '15

The following series implements... --- Addressed comments on V2 and V1 Fixed the while loop exit in P8 virPCIDeviceUnbindFromStub() Shivaprasad G Bhat (9): Implement virPCIIsKnownStub function Initialize the stubDriver of pci devices if bound to a valid one Add iommu group number info to virPCIDevice Refuse to reattach from vfio if the iommu group is in use by any domain Wait for vfio-pci device cleanups before reassinging the device to host driver Split reprobe action from the virPCIUnbindFromStub into a new function Pass activeDevs and inactiveDevs to virPCIDeviceUnbindFromStub and virPCIDeviceBindToStub Postpone reprobing till all the devices in iommu group are unbound from vfio Wait for iommmu device to go away before reprobing the host driver src/libvirt_private.syms | 1 src/util/virhostdev.c | 18 +++ src/util/virpci.c | 300 +++++++++++++++++++++++++++++++++++++++------- src/util/virpci.h | 2 tests/virpcimock.c | 187 ++++++++++++++++++++++++++--- tests/virpcitest.c | 152 +++++++++++++++++++---- 6 files changed, 566 insertions(+), 94 deletions(-) -- Signature

1 9

[libvirt] [PATCH 0/4] *** Don't include .c file for editing support ***
by rodinasophie＠gmail.com 02 Nov '15

02 Nov '15

From: sonya <rodinasophie(a)gmail.com> *** BLURB HERE *** sonya (4): General structures and interfaces were added Virsh-edit interface was added Comment in virsh-edit.h was changed Style-errors are fixed tools/Makefile.am | 1 + tools/virsh-domain.c | 187 ++++++++++++++++++++++++++++++++++++------------ tools/virsh-edit.c | 100 ++++++-------------------- tools/virsh-edit.h | 42 +++++++++++ tools/virsh-interface.c | 64 +++++++++++++---- tools/virsh-network.c | 62 ++++++++++++---- tools/virsh-nwfilter.c | 64 +++++++++++++---- tools/virsh-pool.c | 64 +++++++++++++---- tools/virsh-snapshot.c | 76 ++++++++++++++------ 9 files changed, 464 insertions(+), 196 deletions(-) create mode 100644 tools/virsh-edit.h -- 2.1.4

2 6

[libvirt] [PATCH] syntax-check: Add sc_prohibit_space_in_label rule
by Andrea Bolognani 02 Nov '15

02 Nov '15

This guards against code such as cleanup : which is happily accepted by the compiler but does not conform to our style guidelines. --- cfg.mk | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cfg.mk b/cfg.mk index a9bba38..8462051 100644 --- a/cfg.mk +++ b/cfg.mk @@ -919,6 +919,12 @@ sc_require_space_before_label: halt="Top-level labels should be indented by one space" \ $(_sc_search_regexp) +sc_prohibit_space_in_label: + @prohibit='^[_a-zA-Z0-9]+ +:$$' \ + in_vc_files='\.[ch]$$' \ + halt="There should be no space between label name and colon" \ + $(_sc_search_regexp) + # Doesn't catch all cases of mismatched braces across if-else, but it helps sc_require_if_else_matching_braces: @prohibit='( else( if .*\))? {|} else( if .*\))?$$)' \ -- 2.4.3

2 2