[libvirt] [PATCH] configure: fix check for DEVLINK_CMD_ESWITCH_GET
by Ján Tomko
Instead of checking for all possible constants that every
kernel header with devlink support should have (and defining
HAVE_DECL_DEVLINK as 1 if any of them is present due to the
way AC_CHECK_DECLS works), only check for DEVLINK_CMD_ESWITCH_GET.
This is the name of the constant since kernel 4.11. Between 4.8
and 4.11, the now deprecated spelling DEVLINK_CMD_ESWITCH_MODE_GET
was used.
---
Yet another version.
configure.ac | 9 +++------
src/util/virnetdev.c | 4 ++--
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/configure.ac b/configure.ac
index c9509c7f9..f542359a6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -628,15 +628,12 @@ if test "$with_linux" = "yes"; then
fi
dnl
-dnl check for kernel headers required by devlink
+dnl check for DEVLINK_CMD_ESWITCH_GET, required for checking
+dnl the DEVLINK_ESWITCH_MODE_SWITCHDEV capability
dnl
if test "$with_linux" = "yes"; then
AC_CHECK_HEADERS([linux/devlink.h])
- AC_CHECK_DECLS([DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME, DEVLINK_ATTR_MAX, DEVLINK_CMD_ESWITCH_GET, DEVLINK_ATTR_BUS_NAME, DEVLINK_ATTR_DEV_NAME, DEVLINK_ATTR_ESWITCH_MODE, DEVLINK_ESWITCH_MODE_SWITCHDEV],
- [AC_DEFINE([HAVE_DECL_DEVLINK],
- [1],
- [whether devlink declarations are available])],
- [],
+ AC_CHECK_DECLS([DEVLINK_CMD_ESWITCH_GET], [], [],
[[#include <linux/devlink.h>]])
fi
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 040693925..41a659732 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -59,7 +59,7 @@
# include <net/if_dl.h>
#endif
-#if HAVE_DECL_DEVLINK
+#if HAVE_LINUX_DEVLINK_H
# include <linux/devlink.h>
#endif
@@ -3120,7 +3120,7 @@ virNetDevGetEthtoolFeatures(virBitmapPtr bitmap,
}
-# if HAVE_DECL_DEVLINK
+# if HAVE_DECL_DEVLINK_CMD_ESWITCH_GET
/**
* virNetDevPutExtraHeader
* reserve and prepare room for an extra header
--
2.13.0
7 years, 3 months
[libvirt] [PATCH] configure: Fix devlink detection
by Andrea Bolognani
There are some quirks to detecting whether devlink support can be
activated due to symbols being renamed between Linux versions.
Make detection more robust so that the code can once again compile
on RHEL 7 and others.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
configure.ac | 26 ++++++++++++++++++++++----
src/util/virnetdev.c | 5 +++++
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac
index c9509c7f9..6f03152b4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -632,12 +632,30 @@ dnl check for kernel headers required by devlink
dnl
if test "$with_linux" = "yes"; then
AC_CHECK_HEADERS([linux/devlink.h])
- AC_CHECK_DECLS([DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME, DEVLINK_ATTR_MAX, DEVLINK_CMD_ESWITCH_GET, DEVLINK_ATTR_BUS_NAME, DEVLINK_ATTR_DEV_NAME, DEVLINK_ATTR_ESWITCH_MODE, DEVLINK_ESWITCH_MODE_SWITCHDEV],
- [AC_DEFINE([HAVE_DECL_DEVLINK],
- [1],
- [whether devlink declarations are available])],
+
+ dnl DEVLINK_CMD_ESWITCH_MODE_GET was introduced in Linux 4.8, but Linux
+ dnl 4.11 renamed it to DEVLINK_CMD_ESWITCH_GET. We can use either, but
+ dnl we need at least one of them to be available
+ have_eswitch_get=0
+ AC_CHECK_DECLS([DEVLINK_CMD_ESWITCH_GET, DEVLINK_CMD_ESWITCH_MODE_GET],
+ [have_eswitch_get=1],
+ [],
+ [[#include <linux/devlink.h>]])
+
+ dnl We use a bunch of other symbols as well
+ have_all_others=1
+ AC_CHECK_DECLS([DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME, DEVLINK_ATTR_MAX, DEVLINK_ATTR_BUS_NAME, DEVLINK_ATTR_DEV_NAME, DEVLINK_ATTR_ESWITCH_MODE, DEVLINK_ESWITCH_MODE_SWITCHDEV],
[],
+ [have_all_others=0],
[[#include <linux/devlink.h>]])
+
+ dnl If we have at least one variation of DEVLINK_CMD_ESWITCH_GET *and*
+ dnl all other symbols, then we can enable the devlink code
+ if test have_eswitch_get = 1 && test have_all_others = 1; then
+ AC_DEFINE_UNQUOTED([HAVE_DECL_DEVLINK],
+ [1],
+ [whether devlink declarations are available])
+ fi
fi
dnl Allow perl/python overrides
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 040693925..1e0a257e1 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3245,7 +3245,12 @@ virNetDevSwitchdevFeature(const char *ifname,
if (!(gmsgh = virNetDevPutExtraHeader(nlmsg_hdr(nl_msg), sizeof(struct genlmsghdr))))
goto cleanup;
+#if HAVE_DEVLINK_CMD_ESWITCH_GET
gmsgh->cmd = DEVLINK_CMD_ESWITCH_GET;
+#elif HAVE_DEVLINK_CMD_ESWITCH_MODE_GET
+ gmsgh->cmd = DEVLINK_CMD_ESWITCH_MODE_GET;
+#endif
+
gmsgh->version = DEVLINK_GENL_VERSION;
pci_device_ptr = pfname ? virNetDevGetPCIDevice(pfname) :
--
2.13.5
7 years, 3 months
[libvirt] [PATCH] util: Fix configure.ac check for DEVLINK_CMD_ESWITCH_GET
by John Ferlan
Rather than checking for whether the devlink.h on the system has
multiple symbols, let's only check for whether the command we want
is defined.
Turns out the mechanism of providing multiple definitions to check via
AC_CHECK_DECLS in order to determine whether HAVE_DECL_DEVLINK should
be set resulted in a false positive since as long as one of the defs
was true, then the HAVE_DECL_DEVLINK got defined.
This is further complicated by a change between kernel 4.8 and 4.11
where the originally defined name DEVLINK_CMD_ESWITCH_MODE_GET was
changed to DEVLINK_CMD_ESWITCH_GET with the comment/caveat that
the old format is obsolete should never be used. Therefore, even
though some kernels will have a couple of the same symbols that
were added at the same time (DEVLINK_ATTR_ESWITCH_MODE and
DEVLINK_ESWITCH_MODE_SWITCHDEV), they won't have the newer name
and thus cause a build failure.
So even though multiple DEVLINK_CMD_SWITCH* symbols are used to
determine whether the set HAVE_DECL_DEVLINK, this should cover
those kernels before 4.11 with the old definition.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
I'd push this as a build breaker, but I don't want to need to go through
the trouble again if i got this wrong, so hopefully someone who's seeing
this can try out the patch... It's also present on a couple of the CI
infrastructure environments (f23, centos-7):
https://ci.centos.org/view/libvirt/job/libvirt-master-build/
configure.ac | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index c9509c7f9..73ae7fdd5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -630,12 +630,16 @@ fi
dnl
dnl check for kernel headers required by devlink
dnl
+dnl kernel 4.8 introduced DEVLINK_CMD_ESWITCH_MODE_GET, but that was
+dnl later changed in kernel 4.11 to be just DEVLINK_CMD_ESWITCH_GET, so
+dnl for "completeness" let's allow HAVE_DECL_DEVLINK to be define if
+dnl either is defined.
if test "$with_linux" = "yes"; then
AC_CHECK_HEADERS([linux/devlink.h])
- AC_CHECK_DECLS([DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME, DEVLINK_ATTR_MAX, DEVLINK_CMD_ESWITCH_GET, DEVLINK_ATTR_BUS_NAME, DEVLINK_ATTR_DEV_NAME, DEVLINK_ATTR_ESWITCH_MODE, DEVLINK_ESWITCH_MODE_SWITCHDEV],
+ AC_CHECK_DECLS([DEVLINK_CMD_ESWITCH_GET, DEVLINK_CMD_ESWITCH_MODE_GET],
[AC_DEFINE([HAVE_DECL_DEVLINK],
[1],
- [whether devlink declarations are available])],
+ [whether devlink CMD_ESWITCH_GET is available])],
[],
[[#include <linux/devlink.h>]])
fi
--
2.13.5
7 years, 3 months
[libvirt] [PATCH v2 00/12] Continue altering storage pool for privatization
by John Ferlan
Consider this round 1 of 2.... The next series will be 18 patches,
but the majority of those deal with change every {pool|obj}->def->X
to use the accessor virStoragePoolObjGetDef.
v1: https://www.redhat.com/archives/libvir-list/2017-May/msg00218.html
Probably not even worth looking at the v1, but this picks up where v1
left off somewhere around patch 10, but adding smaller steps between
patches.
John Ferlan (12):
storage: Create accessor API's for virStoragePoolObj
storage: Introduce virStoragePoolObjNew
storage: Fill in storage pool @active properly
storage: Introduce storage volume add, delete, count APIs
storage: Introduce APIs to search/scan storage pool volumes list
storage: Use virStoragePoolObj{Get|Set}ConfigFile
storage: Use virStoragePoolObjGetAutostartLink
storage: Use virStoragePoolObj{Is|Set}Active
storage: Use virStoragePoolObj{Is|Set}Autostart
storage: Internally represent @autostart to bool
storage: Use virStoragePoolObj{Get|Incr}Decr}Asyncjobs
storage: Use virStoragePoolObjDefUseNewDef
src/conf/virstorageobj.c | 211 +++++++++++++++++++++++++++++++--
src/conf/virstorageobj.h | 84 ++++++++++++-
src/libvirt_private.syms | 20 ++++
src/storage/storage_backend_disk.c | 93 +++++++++------
src/storage/storage_backend_gluster.c | 5 +-
src/storage/storage_backend_logical.c | 4 +-
src/storage/storage_backend_mpath.c | 3 +-
src/storage/storage_backend_rbd.c | 4 +-
src/storage/storage_backend_scsi.c | 4 +-
src/storage/storage_backend_sheepdog.c | 4 +-
src/storage/storage_backend_zfs.c | 6 +-
src/storage/storage_driver.c | 142 +++++++++-------------
src/storage/storage_util.c | 8 +-
src/test/test_driver.c | 54 ++++-----
tests/storagevolxml2argvtest.c | 20 ++--
15 files changed, 465 insertions(+), 197 deletions(-)
--
2.9.5
7 years, 3 months
[libvirt] [PATCH] check-spacing: Don't hardcode Perl interpreter path
by Andrea Bolognani
This is particularly useful on operating systems that don't ship
Perl as part of the base system (eg. FreeBSD) while still working
just as well as it did before on Linux.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
build-aux/check-spacing.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/build-aux/check-spacing.pl b/build-aux/check-spacing.pl
index 448acf234..ca8b43491 100755
--- a/build-aux/check-spacing.pl
+++ b/build-aux/check-spacing.pl
@@ -1,4 +1,4 @@
-#!/usr/bin/perl
+#!/usr/bin/env perl
#
# check-spacing.pl: Report any usage of 'function (..args..)'
# Also check for other syntax issues, such as correct use of ';'
--
2.13.5
7 years, 3 months
[libvirt] [PATCH] virsh: man: Describe the 'create' command a bit more
by Erik Skultety
So we refer to the terms 'persistent' and 'transient' across the whole
man page, without describing it further, but more importantly, how the
create command affects it, i.e. explicitly stating that domain created
via the 'create' command are going to be transient or persistent,
depending on whether there is an existing persistent domain with a
matching <name> and <uuid>, in which case it will remain persistent, but
will run using a one-time configuration, otherwise it's going to be
transient and will vanish once destroyed.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
tools/virsh.pod | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 01453be60..c4c76fcb1 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -640,9 +640,14 @@ sessions, such as in a case of a broken connection.
Create a domain from an XML <file>. Optionally, I<--validate> option can be
passed to validate the format of the input XML file against an internal RNG
-schema (identical to using L<virt-xml-validate(1)> tool). An easy way to create
-the XML <file> is to use the B<dumpxml> command to obtain the definition of a
-pre-existing guest. The domain will be paused if the I<--paused> option is used
+schema (identical to using L<virt-xml-validate(1)> tool). Domains created using
+this command are going to be either transient (temporary ones that will vanish
+once destroyed) or existing persistent domains that will run with one-time use
+configuration, leaving the persistent XML untouched (this can come handy during
+an automated testing of various configurations all based on the original XML).
+See the B<Example> section for usage demonstration.
+
+The domain will be paused if the I<--paused> option is used
and supported by the driver; otherwise it will be running. If I<--console> is
requested, attach to the console after creation.
If I<--autodestroy> is requested, then the guest will be automatically
@@ -656,9 +661,24 @@ is only supported with container based virtualization.
B<Example>
- virsh dumpxml <domain> > domain.xml
- vi domain.xml (or make changes with your other text editor)
- virsh create domain.xml
+ 1) prepare a template from an existing domain (skip directly to 3a if writing
+ one from scratch)
+
+ # virsh dumpxml <domain> > domain.xml
+
+ 2) edit the template using an editor of your choice and:
+ a) DO CHANGE! <name> and <uuid> (<uuid> can also be removed), or
+ b) DON'T CHANGE! either <name> or <uuid>
+
+ # $EDITOR domain.xml
+
+ 3) create a domain from domain.xml, depending on whether following 2a or 2b
+ respectively:
+ a) the domain is going to be transient
+ b) an existing persistent domain will run with a modified one-time
+ configuration
+
+ # virsh create domain.xml
=item B<define> I<FILE> [I<--validate>]
--
2.13.3
7 years, 3 months
[libvirt] [PATCH 0/4] qemu: fix restore domain with bypass cache flag
by Nikolay Shirokovskiy
It is not working currently. Error message is (with convinient wraps):
error: Failed to restore domain from tst6.sav
error: internal error: Child process (LIBVIRT_LOG_OUTPUTS=1:stderr \
/usr/libexec/libvirt_iohelper /vz/dev/libvirt/tst6.sav 0 0) unexpected exit status 1: \
/usr/libexec/libvirt_iohelper: failure with /vz/dev/libvirt/tst6.sav : \
Unable to read /vz/dev/libvirt/tst6.sav: Invalid argument
The problem is that saferead function being used by iohelper is not quite suitable
for direct reads. It's last read that is supposed to read EOF is not properly
aligned thus EINVAL is returned.
Nikolay Shirokovskiy (4):
iohelper: drop unused operation length limit
iohelper: simplify last direct write alignment
iohelper: reduce zero-out in align case
iohelper: fix reading with O_DIRECT
src/util/iohelper.c | 98 +++++++++++++++++++++++++++++++----------------------
src/util/virfile.c | 2 +-
2 files changed, 59 insertions(+), 41 deletions(-)
--
1.8.3.1
7 years, 3 months
[libvirt] How to implement pool support in virt-aa-helper?
by Christian Ehrhardt
Hi,
Currently virt-aa-helper has no support for pools, so if you use a volume
from a pool like:
<disk type='volume' device='disk'>
<driver name='qemu' type='raw' cache='none'/>
<source pool='internal' volume='foo'/>
<target dev='vdc' bus='virtio'/>
</disk>
Then there is no matching apparmor rule generated to allow qemu to access
the related devices.
That goes along a simple pool like:
# create a zpool to use
$ for i in $(seq 1 3); do dd if=/dev/zero of=/tmp/fdisk${i} bs=1M
count=1024; done
$ sudo zpool create internal /tmp/fdisk*
# make pool in libvirt and guest disk foo
$ virsh pool-define-as internal zfs
$ virsh pool-start internal
$ virsh vol-create-as internal foo 2G
When libvirt actually builds up the command for qemu what to use based on
the pools information it uses the function
"virStorageTranslateDiskSourcePool" to learn about the actual path of the
disk.
Usually virt-aa-helper tries to stay close to what libvirt does to spawn
the guest, because only then the generated rules match what will be used.
But in this case this isn't as easy.
The lookup by "virStorageTranslateDiskSourcePool" is done on a
virConnectPtr via:
conn->storageDriver->storagePoolLookupByName
That is fine from the code that sets up the guest, but virt-aa-helper has
no virConnectPtr context.
Surely virt-aa-helper could connect via e.g. "virConnectOpenReadOnly" but
the lifecycle of virt-aa-helper doesn't match that. It is invoked as
external program by libvirtd local to the execution of the guest e.g. the
qemu. So adding a connection via a socket makes not much sense. Also in
this environment the connect URI is not set and also not passed to
virt-aa-helper so we can't be sure we have "the same".
I found that while trying to implement that, but I don't like the
preliminary change [1] for the reasons I identified and outlined above. It
initializes the storage drivers correctly, but then fails to look up [2]
and since I doubt it is the right way for the environment and lifecycle of
virt-aa-helper I don't want to go down that way before having a discussion
about it.
Therefore I wanted to ask for some guidance:
1. is there a reasonable way to be able to call
virStorageTranslateDiskSourcePool from virt-aa-helper
2. maybe I overlook a path to get to a valid virConnectPtr from the
virDomainDefPtr or any other in [3] that virt-aa-helper aready has.
3. what would be an alternative to be able to translate pool/volume info to
real paths from virt-aa-helper
[1]: http://paste.ubuntu.com/25570670/
[2]: http://paste.ubuntu.com/25570673/
[3]: http://paste.ubuntu.com/25570720/
--
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
7 years, 3 months
[libvirt] [PATCH] qemu: Introduce a wrapper over virFileWrapperFdClose
by Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=1448268
When migrating to a file (e.g. when doing 'virsh save file'),
couple of things are happening in the thread that is executing
the API:
1) the domain obj is locked
2) iohelper is spawned as a separate process to handle all I/O
3) the thread waits for iohelper to finish
4) the domain obj is unlocked
Now, the problem is that while the thread waits in step 3 for
iohelper to finish this may take ages because iohelper calls
fdatasync(). And unfortunately, we are waiting the whole time
with the domain locked. So if another thread wants to jump in and
say copy the domain name ('virsh list' for instance), they are
stuck.
The solution is to unlock the domain whenever waiting for I/O and
lock it back again when it finished.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b334cf20b..f81d3def4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3216,6 +3216,25 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid,
goto cleanup;
}
+
+static int
+qemuFileWrapperFDClose(virDomainObjPtr vm,
+ virFileWrapperFdPtr fd)
+{
+ int ret;
+
+ /* virFileWrapperFd uses iohelper to write data onto disk.
+ * However, iohelper calls fdatasync() which may take ages to
+ * finish. Therefore, we shouldn't be waiting with the domain
+ * object locked. */
+
+ virObjectUnlock(vm);
+ ret = virFileWrapperFdClose(fd);
+ virObjectLock(vm);
+ return ret;
+}
+
+
/* Helper function to execute a migration to file with a correct save header
* the caller needs to make sure that the processors are stopped and do all other
* actions besides saving memory */
@@ -3276,7 +3295,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
goto cleanup;
}
- if (virFileWrapperFdClose(wrapperFd) < 0)
+ if (qemuFileWrapperFDClose(vm, wrapperFd) < 0)
goto cleanup;
if ((fd = qemuOpenFile(driver, vm, path, O_WRONLY, NULL, NULL)) < 0 ||
@@ -3827,7 +3846,7 @@ doCoreDump(virQEMUDriverPtr driver,
path);
goto cleanup;
}
- if (virFileWrapperFdClose(wrapperFd) < 0)
+ if (qemuFileWrapperFDClose(vm, wrapperFd) < 0)
goto cleanup;
ret = 0;
@@ -6687,7 +6706,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, data, path,
false, QEMU_ASYNC_JOB_START);
- if (virFileWrapperFdClose(wrapperFd) < 0)
+ if (qemuFileWrapperFDClose(vm, wrapperFd) < 0)
VIR_WARN("Failed to close %s", path);
qemuProcessEndJob(driver, vm);
@@ -6962,7 +6981,7 @@ qemuDomainObjRestore(virConnectPtr conn,
ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, data, path,
start_paused, asyncJob);
- if (virFileWrapperFdClose(wrapperFd) < 0)
+ if (qemuFileWrapperFDClose(vm, wrapperFd) < 0)
VIR_WARN("Failed to close %s", path);
cleanup:
--
2.13.5
7 years, 3 months
