[libvirt] [PATCH] Revert "utils: Remove the logging of errors from virNetDevSendEthtoolIoctl"
by Daniel P. Berrange
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
9 years, 5 months
[libvirt] [PATCH] virkeymaps.h: Fix VPATH build with --disable-dependency-tracking
by Michal Privoznik
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
9 years, 5 months
[libvirt] [PATCH] bhyve: add bootrom support
by Roman Bogorodskiy
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
9 years, 5 months
[libvirt] Ten years of libvirt
by Michal Privoznik
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
9 years, 5 months
Re: [libvirt] [PATCH 7/8] Postpone reprobing till all the devices in iommu group are unbound from vfio
by Andrea Bolognani
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
9 years, 5 months
[libvirt] [PATCH v6 00/13] qemu: Add quorum support to libvirt
by Matthias Gatto
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
9 years, 5 months
[libvirt] [PATCH v3 0/9] Few VFIO related fixes
by Shivaprasad G Bhat
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
9 years, 5 months
[libvirt] [PATCH 0/4] *** Don't include .c file for editing support ***
by rodinasophie＠gmail.com
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
9 years, 5 months
[libvirt] [PATCH] syntax-check: Add sc_prohibit_space_in_label rule
by Andrea Bolognani
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
9 years, 5 months
[libvirt] [PATCH v2 0/9] Few VFIO related fixes
by Shivaprasad G Bhat
The following series fixes few VFIO related host crashes during hotplug/unplug
or bind/unbind.
---
Shivaprasad G Bhat (9):
Implement virPCIIsAKnownStub 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 | 289 ++++++++++++++++++++++++++++++++++++++--------
src/util/virpci.h | 2
tests/virpcimock.c | 187 +++++++++++++++++++++++++++---
tests/virpcitest.c | 152 ++++++++++++++++++++----
6 files changed, 555 insertions(+), 94 deletions(-)
--
Signature
9 years, 5 months
