July 2020 - Devel - libvirt List Archives

[RFC] Dynamic creation of VFs in a network definition containing an SRIOV device
by Paulo de Rezende Pinatti 29 Jul '20

29 Jul '20

Context: Libvirt can already detect the active VFs of an SRIOV PF device specified in a network definition and automatically assign these VFs to guests via an <interface> entry referring to that network in the domain definition. This functionality, however, depends on the system administrator having activated in advance the desired number of VFs outside of libvirt (either manually or through system scripts). It would be more convenient if the VFs activation could also be managed inside libvirt so that the whole management of the VF pool is done exclusively by libvirt and in only one place (the network definition) rather than spread in different components of the system. Proposal: We can extend the existing network definition by adding a new tag <vf> as a child of the tag <pf> in order to allow the user to specify how many VFs they wish to have activated for the corresponding SRIOV device when the network is started. That would look like the following: <network> <name>sriov-pool</name> <forward mode='hostdev' managed='yes'> <pf dev='eth1'> <vf num='10'/> </pf> </forward> </network> At xml definition time nothing gets changed on the system, as it is today. When the network is started with 'virth net-start sriov-pool' then libvirt will activate the desired number of VFs as specified in the tag <vf> of the network definition. The operation might require resetting 'sriov_numvfs' to zero first in case the number of VFs currently active differs from the desired value. In order to avoid the situation where the user tries to start the network when a VF is already assigned to a running guest, the implementation will have to ensure all existing VFs of the target PF are not in use, otherwise VFs would be inadvertently hot-unplugged from guests upon network start. In such cases, trying to start the network will then result in an error. Stopping the network with 'virsh net-destroy' will cause all VFs to be removed. Similarly to when starting the network, the implementation will also need to verify for running guests in order to prevent inadvertent hot-unplugging. Is the functionality proposed above desirable? -- Thanks and best regards, Paulo de Rezende Pinatti

3 2

[libvirt PATCH 0/6] tests: qemu: use more g_auto
by Ján Tomko 29 Jul '20

29 Jul '20

Ján Tomko (6): tests: qemu: reduce scope of some variables tests: qemucapsxml2xmltest: split variable declaration tests: qemu: use g_autofree where possible tests: qemu: use g_autoptr where possible tests: qemu: use VIR_AUTOSTRINGLIST where possible tests: qemu: remove unnecessary labels tests/qemuagenttest.c | 16 +++---- tests/qemublocktest.c | 2 +- tests/qemucapabilitiestest.c | 41 ++++++----------- tests/qemucaps2xmltest.c | 39 ++++++---------- tests/qemudomainsnapshotxml2xmltest.c | 29 +++++------- tests/qemuhotplugtest.c | 52 +++++++--------------- tests/qemumemlocktest.c | 26 +++-------- tests/qemumigparamstest.c | 62 +++++++++----------------- tests/qemumonitortestutils.c | 64 ++++++++++----------------- tests/qemusecuritymock.c | 12 ++--- tests/qemuxml2argvtest.c | 27 ++++------- tests/qemuxml2xmltest.c | 9 ++-- tests/testutilsqemu.c | 11 ++--- tests/utiltest.c | 15 +------ 14 files changed, 133 insertions(+), 272 deletions(-) -- 2.26.2

2 7

[libvirt PATCH 0/5] tests: commandtest: use g_auto more
by Ján Tomko 29 Jul '20

29 Jul '20

Ján Tomko (5): tests: commandtest: remove unused 'prefix' parameter tests: commandtest: use g_autofree tests: commandtest: use g_autoptr for virCommand tests: commandtest: use VIR_AUTOCLOSE tests: commandtest: drop unnecessary labels tests/commandtest.c | 327 ++++++++++++++------------------------------ 1 file changed, 105 insertions(+), 222 deletions(-) -- 2.26.2

2 6

[PATCH] doc: fix name of file containing max number of VFs
by Paulo de Rezende Pinatti 28 Jul '20

28 Jul '20

Signed-off-by: Paulo de Rezende Pinatti <ppinatti(a)linux.ibm.com> --- docs/formatnode.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index e4328fedbe..8a51c4da80 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -141,7 +141,7 @@ In this case this device is an SRIOV PF, and the capability element will have a list of <code>address</code> subelements, one for each VF on this PF. If the host system - supports reporting it (via the "sriov_maxvfs" file in the + supports reporting it (via the "sriov_totalvfs" file in the device's sysfs directory) the capability element will also have an attribute named <code>maxCount</code> which is the maximum number of SRIOV VFs supported by this device, which -- 2.26.2

5 4

[PATCH 0/5] docs: misc docs enhancements for statistic API
by Nikolay Shirokovskiy 28 Jul '20

28 Jul '20

Nikolay Shirokovskiy (5): docs: fix typo in virsh.rst for balloon.major_fault include: clarify docs for hugetlb in virDomainMemoryStatTags docs: add missing balloon stats docs in domstats docs: add missing iothread stats docs in domstats src: add missing balloon stats docs docs/manpages/virsh.rst | 11 ++++++++++- include/libvirt/libvirt-domain.h | 4 ++-- src/libvirt-domain.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) -- 1.8.3.1

2 9

[PATCH 0/5] be consistent about error checking xmlNodeGetContent() return
by Laine Stump 28 Jul '20

28 Jul '20

Awhile back I noticed that calls to xmlNodeGetContent() from libvirt code were inconsistent in their handling of the returned pointer. Sometimes we would assume the return was always non-NULL (dereferencing with wild abandon without concern for the consequences), sometimes we would interpret NULL as "", and sometimes as OOM. I sent mail about this to the list last week, wondering (and doubting) if we could assume that a NULL return would always mean OOM: https://www.redhat.com/archives/libvir-list/2020-July/msg00333.html After looking at the libxml code, danpb's determination was that a NULL return from xmlNodeGetContent *might* mean OOM, but it might also mean some odd XML that we weren't expecting, so we can't just always exit on a NULL return. He also agreed with (and expanded on) my suspicion that there really is no reliable way to tell the reason for a NULL return from xmlNodeGetContent, and suggested that possibly we could just examing the xmlNode directly to learn the content instead of calling xmlNodeGetContent(). This series is a followup to that discussion. The first 4 patches clean up the code with the result that: 1) a libvirt wrapper function is always called instead of calling xmlNodeGetContent() directly. 2) that wrapper function logs an error when it gets back NULL from xmlNodeGetContent(). 3) All the callers check for a NULL return, and do a "silent error return" themselves when there is a NULL. In the final patch, I try out Dan's idea of looking at the xmlNode object directly to get the content. It turns out it's not as straightforward as you would think from just looking at the layout of the object - a full explanation is in patch 5. I'm mainly sending that patch as an "FYI" (one step back from an "RFC"), since really all it changes is that libvirt will exit on OOM, and log (different, but not any more informative) error messages when the problem isn't OOM. Unless someone has a strong opinion otherwise, I think just the first 4 patches should be applied, and users can just "deal" with the ambiguity in case of error. Laine Stump (5): conf: refactor virDomainBlkioDeviceParseXML to reduce calls to xmlNodeGetContent util: replace all calls to xmlNodeGetContent with virXMLNodeContentString util: log an error if virXMLNodeContentString will return NULL treat all NULL returns from virXMLNodeContentString() as an error util: open code virXMLNodeContentString to access the node object directly src/conf/domain_conf.c | 194 ++++++++++++++++++++---------------- src/conf/network_conf.c | 7 +- src/conf/node_device_conf.c | 6 +- src/util/virxml.c | 53 +++++++++- 4 files changed, 169 insertions(+), 91 deletions(-) -- 2.25.4

2 7

[libvirt PATCH 0/9] Remove some unused macros
by Ján Tomko 28 Jul '20

28 Jul '20

I built with -Wunused-macros again after five years [0] the good news is we don't really have that many - most of them are VIR_FROM_THIS or macros we have included for completeness. https://gitlab.com/jano.tomko/libvirt/-/pipelines/171383854 [0] a0482396d7e0b164e71b2bdd881d9db5c6521303 Ján Tomko (9): locking: sanlock: drop unused LOCKSPACE_SLEEP storage: logical: drop unused PV_BLANK_SECTOR_SIZE storage: scsi: remove unused LINUX_SYSFS_SCSI_HOST_POSTFIX test: remove unused NUM_CELLS util: remove unused VIR_MCAST_ADDR_LEN virsh: remove unused FILTER macro virt-admin: remove unused VIRT_ADMIN_TIME_BUFLEN util: vportprofile: remove unused constants util: netdevip: remove unused VIR_NETDEV_FAMILY src/locking/lock_driver_sanlock.c | 2 -- src/storage/storage_backend_logical.c | 2 -- src/storage/storage_backend_scsi.c | 1 - src/test/test_driver.c | 2 -- src/util/virnetdev.c | 1 - src/util/virnetdevip.c | 5 ----- src/util/virnetdevvportprofile.c | 4 ---- tools/virsh-network.c | 4 ---- tools/virt-admin.c | 3 --- 9 files changed, 24 deletions(-) -- 2.26.2

2 10

[libvirt PATCH] ci: move sanity_checks after other stages
by Ján Tomko 28 Jul '20

28 Jul '20

Be less hostile to developers and run the build jobs even if the dco-check failed. That way they can test their own "private" branches without a sign-off. Also specify empty `needs` for it, since it does not depend on any of the jobs in the container phase and can be run right away. Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- A failed build still runs DCO: https://gitlab.com/jano.tomko/libvirt/-/pipelines/171565689 and vice-versa: https://gitlab.com/jano.tomko/libvirt/-/pipelines/171582293 .gitlab-ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 702198ec8e..f97c96dd21 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -2,9 +2,9 @@ variables: GIT_DEPTH: 100 stages: - - sanity_checks - containers - builds + - sanity_checks .script_variables: &script_variables | export MAKEFLAGS="-j$(getconf _NPROCESSORS_ONLN)" @@ -481,6 +481,7 @@ potfile: # merge requests are submitted check-dco: stage: sanity_checks + needs: [] image: registry.gitlab.com/libvirt/libvirt-ci/check-dco:master script: - /check-dco -- 2.26.2

2 1

[PATCH] qemu: modprobe vhost-vsock module
by Nikolay Shirokovskiy 28 Jul '20

28 Jul '20

/dev/vhost-vsock is usable only if the module is loaded. Let's load the module just like in other places where kernel module is required (nbd, pci stub driver). Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com> --- src/qemu/qemu_process.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ec6ca14..5aaa77c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -92,6 +92,7 @@ #include "viridentity.h" #include "virthreadjob.h" #include "virutil.h" +#include "virkmod.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -6414,6 +6415,7 @@ qemuProcessPrepareHostStorage(virQEMUDriverPtr driver, return 0; } +#define VHOST_VSOCK_MODULE "vhost-vsock" int qemuProcessOpenVhostVsock(virDomainVsockDefPtr vsock) @@ -6422,6 +6424,22 @@ qemuProcessOpenVhostVsock(virDomainVsockDefPtr vsock) const char *vsock_path = "/dev/vhost-vsock"; int fd; + if (virKModIsProhibited(VHOST_VSOCK_MODULE)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to load vhost-vsock module: " + "administratively prohibited")); + return -1; + } else { + g_autofree char *errbuf = NULL; + + if ((errbuf = virKModLoad(VHOST_VSOCK_MODULE))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to load vhost-vsock module: %s"), + errbuf); + return -1; + } + } + if ((fd = open(vsock_path, O_RDWR)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("unable to open vhost-vsock device")); -- 1.8.3.1

2 2

[libvirt PATCH] ci: move dco-check to a later stage
by Ján Tomko 28 Jul '20

28 Jul '20

Be less hostile to developers and run the build jobs even if the dco-check failed. That way they can test their own "private" branches without a sign-off. Also specify empty `needs` for it, since it does not depend on any of the jobs in the container phase and can be run right away. Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- https://gitlab.com/jano.tomko/libvirt/-/pipelines/171532004 .gitlab-ci.yml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 702198ec8e..b7a1609580 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -2,7 +2,6 @@ variables: GIT_DEPTH: 100 stages: - - sanity_checks - containers - builds @@ -479,8 +478,15 @@ potfile: # Skip on "libvirt" namespace, since we only need to run # this test on developer's personal forks from which # merge requests are submitted +# +# Put it in the same stage as other tests we expect to fail +# because of developer's changes as to not frustrate them +# with partial feedback, but run it as soon as the pipeline +# starts, since it does not have any `needs` from the previous +# stages. check-dco: - stage: sanity_checks + stage: builds + needs: [] image: registry.gitlab.com/libvirt/libvirt-ci/check-dco:master script: - /check-dco -- 2.26.2

3 2