Hyper-V CPU details
Hello, I'm implementing domainGetVcpus and could use some guidance on what value to use for virVcpuInfo->cpu. Hyper-V does not allow the user to pin vCPUs to host CPUs and doesn't allow the user to see which host CPU a vCPU is currently running on. Since it's a type 1 hypervisor, none of its scheduling data is available to the Windows userspace: there aren't any processes or threads that correspond to vCPUs that I could query the OS scheduler about. My code currently sets it to -1, which produces the following `virsh vcpuinfo` output for a running VM with two cores: VCPU: 0 CPU:-1 State: running CPU time: 1684.5s CPU Affinity: VCPU: 1 CPU:-1 State: running CPU time: 1346.0s CPU Affinity: However, this doesn't match the comment in _virVcpuInfo's declaration, which says that -1 signifies an offline CPU: https://gitlab.com/libvirt/libvirt/-/blob/v6.8.0/include/libvirt/libvirt-domain.h#L1918 Should I stick with -1? Or, should I introduce -2 as a value that indicates that the hypervisor doesn't provide that information? Or, is there some better way to handle this that I'm not aware of? -- Matt
Re: [External] Re: [PATCH v3 0/2]support memory failure
On 10/23/20 3:38 AM, zhenwei pi wrote: On 10/23/20 1:00 AM, Michal Privoznik wrote: On 10/14/20 12:37 PM, zhenwei pi wrote: v2->v3: Rework patches to make each patch could be compiled, v1->v2: Seperate a 'all in one' patch into 4 patches. Use a 'flags' with bit definition instead of 'action_required' & 'recursive' for extention. Queue event directly without internal job. Add full test method in commit. v1: Since QEMU 5.2 (commit-77b285f7f6), QEMU supports 'memory failure' event, posts event to monitor if hitting a hardware memory error. zhenwei pi (2): libvirt: support memory failure event qemu: implement memory failure event include/libvirt/libvirt-domain.h | 82 + src/conf/domain_event.c | 80 src/conf/domain_event.h | 12 ++ src/libvirt_private.syms | 2 + src/remote/remote_daemon_dispatch.c | 32 +++ src/remote/remote_driver.c | 32 +++ src/remote/remote_protocol.x | 16 +++- src/remote_protocol-structs | 8 examples/c/misc/event-test.c | 16 tools/virsh-domain.c | 40 ++ src/qemu/qemu_monitor.c | 21 +- src/qemu/qemu_monitor.h | 39 ++ src/qemu/qemu_monitor_json.c | 49 ++ src/qemu/qemu_process.c | 59 ++ 14 files changed, 486 insertions(+), 2 deletions(-) Sorry for not replying earlier, had this marked for review but got sidetracked with some other work. Patches look good to me, but I'm suggesting couple of fixups. If you agree with them I can put my reviewed by tag and merge them. Michal I agree with these changes. Thanks a lot. Alright, pushed. Can you you please post a follow up patch that adds this new feature to NEWS.rst? Michal
Re: [External] Re: [PATCH v3 0/2]support memory failure
On 10/23/20 3:46 PM, Michal Privoznik wrote: On 10/23/20 3:38 AM, zhenwei pi wrote: On 10/23/20 1:00 AM, Michal Privoznik wrote: On 10/14/20 12:37 PM, zhenwei pi wrote: v2->v3: Rework patches to make each patch could be compiled, v1->v2: Seperate a 'all in one' patch into 4 patches. Use a 'flags' with bit definition instead of 'action_required' & 'recursive' for extention. Queue event directly without internal job. Add full test method in commit. v1: Since QEMU 5.2 (commit-77b285f7f6), QEMU supports 'memory failure' event, posts event to monitor if hitting a hardware memory error. zhenwei pi (2): libvirt: support memory failure event qemu: implement memory failure event include/libvirt/libvirt-domain.h | 82 + src/conf/domain_event.c | 80 src/conf/domain_event.h | 12 ++ src/libvirt_private.syms | 2 + src/remote/remote_daemon_dispatch.c | 32 +++ src/remote/remote_driver.c | 32 +++ src/remote/remote_protocol.x | 16 +++- src/remote_protocol-structs | 8 examples/c/misc/event-test.c | 16 tools/virsh-domain.c | 40 ++ src/qemu/qemu_monitor.c | 21 +- src/qemu/qemu_monitor.h | 39 ++ src/qemu/qemu_monitor_json.c | 49 ++ src/qemu/qemu_process.c | 59 ++ 14 files changed, 486 insertions(+), 2 deletions(-) Sorry for not replying earlier, had this marked for review but got sidetracked with some other work. Patches look good to me, but I'm suggesting couple of fixups. If you agree with them I can put my reviewed by tag and merge them. Michal I agree with these changes. Thanks a lot. Alright, pushed. Can you you please post a follow up patch that adds this new feature to NEWS.rst? OK. Thanks a lot. Michal -- zhenwei pi
[PATCH] news: introduce memory failure event
Signed-off-by: zhenwei pi --- NEWS.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index d0454b7840..a01481801e 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -13,6 +13,11 @@ v6.9.0 (unreleased) * **New features** + * Introduce memory failure event + +Libvirt could handle domain's memory failure event. Drivers need to +implement their own method. Currently, only QEMU supports this feature. + * qemu: Implement support for disks VMs based on the QEMU hypervisor now can use option for -- 2.11.0
[libvirt PATCH] libvirt-guests: Sync time for autostarted guests
From: Orion Poplawski
Setting SYNC_TIME=1 does not work on autostarted guests.
Signed-off-by: Tim Wiederhake
---
tools/libvirt-guests.sh.in | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in
index d69df908d2..87f96af14d 100644
--- a/tools/libvirt-guests.sh.in
+++ b/tools/libvirt-guests.sh.in
@@ -206,9 +206,9 @@ start() {
retval run_virsh "$uri" start $bypass "$name" \
>/dev/null && \
gettext "done"; echo
-if "$sync_time"; then
-run_virsh "$uri" domtime --sync "$name" >/dev/null
-fi
+fi
+if "$sync_time"; then
+run_virsh "$uri" domtime --sync "$name" >/dev/null
fi
fi
done
--
2.26.2
Re: [libvirt PATCH] libvirt-guests: Sync time for autostarted guests
On Fri, Oct 23, 2020 at 11:22:47AM +0200, Tim Wiederhake wrote:
> From: Orion Poplawski
>
> Setting SYNC_TIME=1 does not work on autostarted guests.
>
> Signed-off-by: Tim Wiederhake
We'd expect to see a signed-off-by from the patch author generally.
By putting your own Signed-Off-By here, you are stating *you* believe
that the original author has complied with the DCO. Having the original
author's Signed-Off-By is how you typically satisfy that belief.
> ---
> tools/libvirt-guests.sh.in | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in
> index d69df908d2..87f96af14d 100644
> --- a/tools/libvirt-guests.sh.in
> +++ b/tools/libvirt-guests.sh.in
> @@ -206,9 +206,9 @@ start() {
> retval run_virsh "$uri" start $bypass "$name" \
> >/dev/null && \
> gettext "done"; echo
> -if "$sync_time"; then
> -run_virsh "$uri" domtime --sync "$name" >/dev/null
> -fi
> +fi
> +if "$sync_time"; then
> +run_virsh "$uri" domtime --sync "$name" >/dev/null
> fi
> fi
> done
> --
> 2.26.2
>
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] news: introduce memory failure event
On 10/23/20 10:50 AM, zhenwei pi wrote: Signed-off-by: zhenwei pi --- NEWS.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index d0454b7840..a01481801e 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -13,6 +13,11 @@ v6.9.0 (unreleased) * **New features** + * Introduce memory failure event + +Libvirt could handle domain's memory failure event. Drivers need to +implement their own method. Currently, only QEMU supports this feature. + * qemu: Implement support for disks VMs based on the QEMU hypervisor now can use option for Well, the first sentence makes sense, but the rest is more for us, developers. How about this: * qemu: Implement memory failure event New event is implemented that is emitted whenever a guest encounters a memory failure. Michal
Re: where can i find the sourcecode from virDomainDestroy ?
- On Oct 16, 2020, at 6:34 PM, Laine Stump la...@redhat.com wrote:
I installed the source package from libvirt-4.0.0, i have a SLES 12 SP4.
>
>
> The first thing to try, if there is any possibility, is to upgrade your
> libvirt version. 4.0.0 is nearly 3 years out of date at this time, and
> *a lot* has changed since then. If you actually are experiencing a
> libvirt bug, then it's very likely it was identified and fixed during
> this time. (Even though you are running and older version of SLES, it's
> likely someone somewhere has built a newer version and has a pre-built
> package available)
Yes, but installing a more recent version not from the official SLES repository
makes me loose
my support.
>> I found a virsh-domain.c which i assume is responsible for the domains.
>> Right ?
>
> virsh-domain.c contains the functions that correspond to domain-related
> commands of the virsh utility. They gather input from the commandline,
> then call libvirt's public API. So you won't find much there.
>
This is the part i want to keep an eye on (virsh-domain.c)
:
static bool
cmdDestroy(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom;
bool ret = true;
const char *name;
unsigned int flags = 0;
int result;
if (!(dom = virshCommandOptDomain(ctl, cmd, &name)))
return false;
if (vshCommandOptBool(cmd, "graceful"))
flags |= VIR_DOMAIN_DESTROY_GRACEFUL;
if (flags)
result = virDomainDestroyFlags(dom, VIR_DOMAIN_DESTROY_GRACEFUL);
else
result = virDomainDestroy(dom); <=
if (result == 0) {
vshPrintExtra(ctl, _("Domain %s destroyed\n"), name);
} else {
vshError(ctl, _("Failed to destroy domain %s"), name);
ret = false;
}
virshDomainFree(dom);
return ret;
}
Does that mean that virsh itself returns a true and write "Domain %s
destroyed\n" if it successfully
destroyed the domain and otherwise returns a false and writes "Domain %s
destroyed\n" ?
>> I found a function called "virDomainDestroy", which i believe has the
>> purpose to
>> destroy the domain. Right ?
> That is the top-level public API (what is called by user programs like
> virsh or virt-manager). It looks into the virConnect object that was
...
If i want to check success or not with virsh then the top-level API is enough
for me.
Bernd
Helmholtz Zentrum München
Helmholtz Zentrum Muenchen
Deutsches Forschungszentrum fuer Gesundheit und Umwelt (GmbH)
Ingolstaedter Landstr. 1
85764 Neuherberg
www.helmholtz-muenchen.de
Aufsichtsratsvorsitzende: MinDir.in Prof. Dr. Veronika von Messling
Geschaeftsfuehrung: Prof. Dr. med. Dr. h.c. Matthias Tschoep, Kerstin Guenther
Registergericht: Amtsgericht Muenchen HRB 6466
USt-IdNr: DE 129521671
Re: [PATCH 0/3] Fix stat mocks on macOS
On 10/18/20 5:30 PM, Roman Bolshakov wrote: Hi, The series partially addresses https://gitlab.com/libvirt/libvirt/-/issues/58 by enabling stat mocks and that fixes qemufirmwaretest, domaincapstest and qemuvhostusertest. Thanks, Roman Roman Bolshakov (3): tests: Fix lstat() mock initialization on macOS tests: Re-introduce stat/lstat mocks on macOS tests: Use flat namespace for qemu test driver tests/meson.build | 1 + tests/virmockstathelpers.c | 18 +- 2 files changed, 14 insertions(+), 5 deletions(-) Reviewed-by: Michal Privoznik and pushed. Michal
Re: [PATCH] qemu: virtiofs: Add pool element to limit thread pool
On 10/20/20 2:54 PM, Masayoshi Mizuma wrote:
From: Masayoshi Mizuma
virtiofsd has --thread-pool-size=NUM option to limit the
thread pool size. It will be useful to tune the performance.
Add pool element to use the option like as:
Signed-off-by: Masayoshi Mizuma
---
Code looks good. We generally split the docs from the parser/qemu logic
in the commits though. For new options it's also good to have unit tests
covering them as well.
I suggest you split this patch in at least 2 commits. First commit contains
the changes in the /docs files, introducing the new 'pool' option. The
second commit contains the changes in domain_conf and qemu_virtiofs that
implements it.
For unit tests, you'll want something in the lines of what is done in the
case of 'vhost-user-fs-fd-memory' test in qemuxml2argvtest.c. You'll need
a new XML file (e.g. vhost-user-fs-pool.xml) in qemuxml2argvdata that
implements the 'pool' option, then a .args file that will contains the
expected QEMU command line generated by that XML. You can see how it is being
done for 'vhost-user-fs-fd-memory' and go from there. All the extra files
and changes for this new test goes in another patch.
Last but not the least, if you want to make the maintainers life easier, you
can also add an extra patch documenting this new feature in NEWS.rst.
Thanks,
DHB
docs/formatdomain.rst | 6 --
docs/schemas/domaincommon.rng | 5 +
src/conf/domain_conf.c| 12
src/conf/domain_conf.h| 1 +
src/qemu/qemu_virtiofs.c | 3 +++
5 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index bfa80e4bc2..0f66af5dc3 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3070,7 +3070,7 @@ A directory on the host that can be accessed directly
from the guest.
-
+
@@ -3188,7 +3188,9 @@ A directory on the host that can be accessed directly
from the guest.
the use of filesystem extended attributes. Caching can be tuned via the
``cache`` element, possible ``mode`` values being ``none`` and ``always``.
Locking can be controlled via the ``lock`` element - attributes ``posix``
and
- ``flock`` both accepting values ``on`` or ``off``. ( :since:`Since 6.2.0` )
+ ``flock`` both accepting values ``on`` or ``off``. ( :since:`Since 6.2.0` ).
+ Thread pool size limit can be tuned via the ``pool`` element.
+ ( :since:`Since 6.9.0` )
``source``
The resource on the host that is being accessed in the guest. The ``name``
attribute must be used with ``type='template'``, and the ``dir`` attribute
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c25a742581..cd5de2ba80 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2839,6 +2839,11 @@
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index efa5ac527b..9d06f8c75f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11588,6 +11588,7 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
g_autofree char *cache =
virXPathString("string(./binary/cache/@mode)", ctxt);
g_autofree char *posix_lock =
virXPathString("string(./binary/lock/@posix)", ctxt);
g_autofree char *flock =
virXPathString("string(./binary/lock/@flock)", ctxt);
+g_autofree char *thread_pool_size =
virXPathString("string(./binary/@pool)", ctxt);
int val;
if (queue_size && virStrToLong_ull(queue_size, NULL, 10, &def->queue_size) < 0) {
@@ -11597,6 +11598,14 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
}
+if (thread_pool_size &&
+virStrToLong_ull(thread_pool_size, NULL, 10, &def->thread_pool_size)
< 0) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("cannot parse thread pool size '%s' for
virtiofs"),
+ thread_pool_size);
+goto error;
+}
+
if (binary)
def->binary = virFileSanitizePath(binary);
@@ -26191,6 +26200,9 @@ virDomainFSDefFormat(virBufferPtr buf,
virTristateSwitchTypeToString(def->xattr));
}
+if (def->thread_pool_size)
+virBufferAsprintf(&binaryAttrBuf, " pool='%llu'",
def->thread_pool_size);
+
if (def->cache != VIR_DOMAIN_FS_CACHE_MODE_DEFAULT) {
virBufferAsprintf(&binaryBuf, "\n",
virDomainFSCacheModeTypeToString(def->cache));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index cd344716a3..98ab0a51c7 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -860,6 +860,7 @@ struct _vi
Re: [libvirt PATCH] qemu: honour fatal errors dealing with qemu slirp helper
On 10/20/20 12:07 PM, Daniel P. Berrangé wrote:
Currently all errors from qemuInterfacePrepareSlirp() are completely
ignored by the callers. The intention is that missing qemu-slirp binary
should cause the caller to fallback to the built-in slirp impl.
Many of the possible errors though should indeed be considered fatal.
Signed-off-by: Daniel P. Berrangé
---
Just a FYI: there is a trivial conflict in qemu_interface.h when applying
it to current master.
Reviewed-by: Daniel Henrique Barboza
src/qemu/qemu_hotplug.c | 7 +--
src/qemu/qemu_interface.c | 21 +++--
src/qemu/qemu_interface.h | 5 +++--
src/qemu/qemu_process.c | 8 ++--
src/qemu/qemu_slirp.c | 3 ---
5 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 79fc8baa5c..dc998236de 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1311,9 +1311,12 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
case VIR_DOMAIN_NET_TYPE_USER:
if (!priv->disableSlirp &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) {
-qemuSlirpPtr slirp = qemuInterfacePrepareSlirp(driver, net);
+qemuSlirpPtr slirp = NULL;
+int rv = qemuInterfacePrepareSlirp(driver, net, &slirp);
-if (!slirp)
+if (rv == -1)
+return -1;
+if (rv == 0)
break;
QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp = slirp;
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index cbf3d99981..b4ab809970 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -636,30 +636,39 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def,
}
-qemuSlirpPtr
+/*
+ * Returns: -1 on error, 0 if slirp isn't available, 1 on succcess
+ */
+int
qemuInterfacePrepareSlirp(virQEMUDriverPtr driver,
- virDomainNetDefPtr net)
+ virDomainNetDefPtr net,
+ qemuSlirpPtr *slirpret)
{
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autoptr(qemuSlirp) slirp = NULL;
size_t i;
+if (!cfg->slirpHelperName ||
+!virFileExists(cfg->slirpHelperName))
+return 0; /* fallback to builtin slirp impl */
+
if (!(slirp = qemuSlirpNewForHelper(cfg->slirpHelperName)))
-return NULL;
+return -1;
for (i = 0; i < net->guestIP.nips; i++) {
const virNetDevIPAddr *ip = net->guestIP.ips[i];
if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET) &&
!qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_IPV4))
-return NULL;
+return 0;
if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6) &&
!qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_IPV6))
-return NULL;
+return 0;
}
-return g_steal_pointer(&slirp);
+*slirpret = g_steal_pointer(&slirp);
+return 1;
}
diff --git a/src/qemu/qemu_interface.h b/src/qemu/qemu_interface.h
index 3dcefc6a12..b5e91e3ab2 100644
--- a/src/qemu/qemu_interface.h
+++ b/src/qemu/qemu_interface.h
@@ -56,5 +56,6 @@ int qemuInterfaceOpenVhostNet(virDomainDefPtr def,
int *vhostfd,
size_t *vhostfdSize) G_GNUC_NO_INLINE;
-qemuSlirpPtr qemuInterfacePrepareSlirp(virQEMUDriverPtr driver,
- virDomainNetDefPtr net);
+int qemuInterfacePrepareSlirp(virQEMUDriverPtr driver,
+ virDomainNetDefPtr net,
+ qemuSlirpPtr *slirp);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5bc76a75e3..59206a17fb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5697,9 +5697,13 @@ qemuProcessNetworkPrepareDevices(virQEMUDriverPtr driver,
} else if (actualType == VIR_DOMAIN_NET_TYPE_USER &&
!priv->disableSlirp &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) {
-qemuSlirpPtr slirp = qemuInterfacePrepareSlirp(driver, net);
+qemuSlirpPtr slirp = NULL;
+int rv = qemuInterfacePrepareSlirp(driver, net, &slirp);
-QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp = slirp;
+if (rv == -1)
+return -1;
+if (rv == 1)
+QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp = slirp;
}
}
diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c
index d2e4ed79be..dfb36125f0 100644
--- a/src/qemu/qemu_slirp.c
+++ b/src/qemu/qemu_slirp.c
@@ -101,9 +101,6 @@ qemuSlirpNewForHelper(const char *helper)
virJSONValuePtr featuresJSON;
size_t i, nfeatures;
-if (!helper)
-return NULL;
-
slirp = qemuSlirpNew();
if (!slirp) {
virReportError(VIR_ERR_INTERNAL_ERRO
[PATCH 2/3] example: add ipv6 filters examples
Signed-off-by: Aleksandr Alekseev --- src/nwfilter/xml/allow-dhcpv6-server.xml | 27 src/nwfilter/xml/allow-dhcpv6.xml| 24 + src/nwfilter/xml/allow-incoming-ipv6.xml | 3 +++ src/nwfilter/xml/allow-ipv6.xml | 3 +++ src/nwfilter/xml/meson.build | 6 ++ src/nwfilter/xml/no-ipv6-multicast.xml | 9 src/nwfilter/xml/no-ipv6-spoofing.xml| 15 + 7 files changed, 87 insertions(+) create mode 100644 src/nwfilter/xml/allow-dhcpv6-server.xml create mode 100644 src/nwfilter/xml/allow-dhcpv6.xml create mode 100644 src/nwfilter/xml/allow-incoming-ipv6.xml create mode 100644 src/nwfilter/xml/allow-ipv6.xml create mode 100644 src/nwfilter/xml/no-ipv6-multicast.xml create mode 100644 src/nwfilter/xml/no-ipv6-spoofing.xml diff --git a/src/nwfilter/xml/allow-dhcpv6-server.xml b/src/nwfilter/xml/allow-dhcpv6-server.xml new file mode 100644 index 00..214a95f412 --- /dev/null +++ b/src/nwfilter/xml/allow-dhcpv6-server.xml @@ -0,0 +1,27 @@ + + + + + + + + + + + + + diff --git a/src/nwfilter/xml/allow-dhcpv6.xml b/src/nwfilter/xml/allow-dhcpv6.xml new file mode 100644 index 00..f3512af153 --- /dev/null +++ b/src/nwfilter/xml/allow-dhcpv6.xml @@ -0,0 +1,24 @@ + + + + + + + + + + + + + diff --git a/src/nwfilter/xml/allow-incoming-ipv6.xml b/src/nwfilter/xml/allow-incoming-ipv6.xml new file mode 100644 index 00..93e1b18784 --- /dev/null +++ b/src/nwfilter/xml/allow-incoming-ipv6.xml @@ -0,0 +1,3 @@ + + + diff --git a/src/nwfilter/xml/allow-ipv6.xml b/src/nwfilter/xml/allow-ipv6.xml new file mode 100644 index 00..8da5188cb9 --- /dev/null +++ b/src/nwfilter/xml/allow-ipv6.xml @@ -0,0 +1,3 @@ + + + diff --git a/src/nwfilter/xml/meson.build b/src/nwfilter/xml/meson.build index 95af75bb15..0d96c54ebe 100644 --- a/src/nwfilter/xml/meson.build +++ b/src/nwfilter/xml/meson.build @@ -2,8 +2,12 @@ nwfilter_xml_files = [ 'allow-arp.xml', 'allow-dhcp-server.xml', 'allow-dhcp.xml', + 'allow-dhcpv6-server.xml', + 'allow-dhcpv6.xml', 'allow-incoming-ipv4.xml', + 'allow-incoming-ipv6.xml', 'allow-ipv4.xml', + 'allow-ipv6.xml', 'clean-traffic-gateway.xml', 'clean-traffic.xml', 'no-arp-ip-spoofing.xml', @@ -11,6 +15,8 @@ nwfilter_xml_files = [ 'no-arp-spoofing.xml', 'no-ip-multicast.xml', 'no-ip-spoofing.xml', + 'no-ipv6-multicast.xml', + 'no-ipv6-spoofing.xml', 'no-mac-broadcast.xml', 'no-mac-spoofing.xml', 'no-other-l2-traffic.xml', diff --git a/src/nwfilter/xml/no-ipv6-multicast.xml b/src/nwfilter/xml/no-ipv6-multicast.xml new file mode 100644 index 00..a736366374 --- /dev/null +++ b/src/nwfilter/xml/no-ipv6-multicast.xml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/src/nwfilter/xml/no-ipv6-spoofing.xml b/src/nwfilter/xml/no-ipv6-spoofing.xml new file mode 100644 index 00..a9ca690345 --- /dev/null +++ b/src/nwfilter/xml/no-ipv6-spoofing.xml @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + -- 2.28.0.97.gdc04167d37
[PATCH 0/3] IPV6 filters example.
Add nwfilter examples for ipv6 similar to existing ip filters. Add appropriate docs for them and for some previously undocumented, but existing filters. Also fix a typo and some formatting. Aleksandr Alekseev (3): example: fix typo and formatting example: add ipv6 filters examples doc: document new filters and not documented ones docs/firewall.html.in| 9 ++ docs/formatnwfilter.html.in | 41 ++-- src/nwfilter/xml/allow-dhcp-server.xml | 4 +-- src/nwfilter/xml/allow-dhcp.xml | 4 +-- src/nwfilter/xml/allow-dhcpv6-server.xml | 27 src/nwfilter/xml/allow-dhcpv6.xml| 24 ++ src/nwfilter/xml/allow-incoming-ipv6.xml | 3 ++ src/nwfilter/xml/allow-ipv6.xml | 3 ++ src/nwfilter/xml/meson.build | 6 src/nwfilter/xml/no-ipv6-multicast.xml | 9 ++ src/nwfilter/xml/no-ipv6-spoofing.xml| 15 + 11 files changed, 138 insertions(+), 7 deletions(-) create mode 100644 src/nwfilter/xml/allow-dhcpv6-server.xml create mode 100644 src/nwfilter/xml/allow-dhcpv6.xml create mode 100644 src/nwfilter/xml/allow-incoming-ipv6.xml create mode 100644 src/nwfilter/xml/allow-ipv6.xml create mode 100644 src/nwfilter/xml/no-ipv6-multicast.xml create mode 100644 src/nwfilter/xml/no-ipv6-spoofing.xml -- 2.28.0.97.gdc04167d37
[PATCH 1/3] example: fix typo and formatting
Signed-off-by: Aleksandr Alekseev --- src/nwfilter/xml/allow-dhcp-server.xml | 4 ++-- src/nwfilter/xml/allow-dhcp.xml| 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/nwfilter/xml/allow-dhcp-server.xml b/src/nwfilter/xml/allow-dhcp-server.xml index 37e708ed4b..7fb426a660 100644 --- a/src/nwfilter/xml/allow-dhcp-server.xml +++ b/src/nwfilter/xml/allow-dhcp-server.xml @@ -1,7 +1,7 @@ - - - -
[PATCH 3/3] doc: document new filters and not documented ones
Signed-off-by: Aleksandr Alekseev --- docs/firewall.html.in | 9 docs/formatnwfilter.html.in | 41 ++--- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/docs/firewall.html.in b/docs/firewall.html.in index 62f37e0eea..15b4f397be 100644 --- a/docs/firewall.html.in +++ b/docs/firewall.html.in @@ -283,12 +283,21 @@ UUID Name 15b1ab2b-b1ac-1be2-ed49-2042caba4abb allow-arp 6c51a466-8d14-6d11-46b0-68b1a883d00f allow-dhcp 7517ad6c-bd90-37c8-26c9-4eabcb69848d allow-dhcp-server +7680776c-77aa-496f-90d6-13097664b925 allow-dhcpv6 +9cdaad60-7631-4172-8ccb-ef774be7485b allow-dhcpv6-server 3d38b406-7cf0-8335-f5ff-4b9add35f288 allow-incoming-ipv4 +908543c1-902e-45f6-a6ca-1a0ad35e7599 allow-incoming-ipv6 5ff06320-9228-2899-3db0-e32554933415 allow-ipv4 +ce8904cc-ad3a-4454-896c-53452882f817 allow-ipv6 db0b1767-d62b-269b-ea96-0cc8b451144e clean-traffic +6d6ddcc8-1242-4c43-ac63-63af80493132 clean-traffic-gateway +4cf38077-c7d5-4e25-99bb-6c4c9efad294 no-arp-ip-spoofing +0b11a636-ce58-497f-be90-17f63c92487a no-arp-mac-spoofing f88f1932-debf-4aa1-9fbe-f10d3aa4bc95 no-arp-spoofing 772f112d-52e4-700c-0250-e178a3d91a7a no-ip-multicast 7ee20370-8106-765d-f7ff-8a60d5aaf30b no-ip-spoofing +f8a51c43-a08f-49b3-b9e2-393d54522dc0 no-ipv6-multicast +a7f0afe9-a428-44b8-8566-c8ee2a669271 no-ipv6-spoofing d5d3c490-c2eb-68b1-24fc-3ee362fc8af3 no-mac-broadcast fb57c546-76dc-a372-513f-e8179011b48a no-mac-spoofing dba10ea7-446d-76de-346f-335bd99c1d05 no-other-l2-traffic diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in index 796c16549d..04aeda06ec 100644 --- a/docs/formatnwfilter.html.in +++ b/docs/formatnwfilter.html.in @@ -467,8 +467,7 @@ DSTPORTS = [ 80, 8080 ] IPV6 - Not currently implemented: - the list of IPV6 addresses in use by an interface + The list of IPV6 addresses in use by an interface DHCPSERVER @@ -2011,11 +2010,35 @@ echo 3 > /proc/sys/net/netfilter/nf_conntrack_icmp_timeout only allows ARP request and reply messages and enforces that those packets contain the MAC and IP addresses of the VM. + + + allow-arp + Allow ARP traffic in both directions + + + allow-ipv4 + Allow IPv4 traffic in both directions + + + allow-ipv6 + Allow IPv6 traffic in both directions + + + allow-incoming-ipv4 + Allow incoming IPv4 traffic + + + allow-incoming-ipv6 + Allow incoming IPv6 traffic allow-dhcp Allow a VM to request an IP address via DHCP (from any DHCP server) + + + allow-dhcpv6 + Similar to allow-dhcp, but for DHCPv6 allow-dhcp-server @@ -2023,16 +2046,28 @@ echo 3 > /proc/sys/net/netfilter/nf_conntrack_icmp_timeout DHCP server. The dotted decimal IP address of the DHCP server must be provided in a reference to this filter. The name of the variable must be DHCPSERVER. + + + allow-dhcpv6-server + Similar to allow-dhcp-server, but for DHCPv6 no-ip-spoofing - Prevent a VM from sending of IP packets with + Prevent a VM from sending of IPv4 packets with a source IP address different from the one in the packet. + + + no-ipv6-spoofing + Similar to no-ip-spoofing, but for IPv6 no-ip-multicast Prevent a VM from sending IP multicast packets. + + + no-ipv6-multicast + Similar to no-ip-multicast, but for IPv6 clean-traffic -- 2.28.0.97.gdc04167d37
[PATCH v2] news: introduce memory failure event
Signed-off-by: zhenwei pi --- NEWS.rst | 10 ++ 1 file changed, 10 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index d0454b7840..428928e80b 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -13,6 +13,16 @@ v6.9.0 (unreleased) * **New features** + * Introduce memory failure event + +Libvirt could handle domain's memory failure event. Drivers need to +implement their own method. + + * qemu: Implement memory failure event + +New event is implemented that is emitted whenever a guest encounters a +memory failure. + * qemu: Implement support for disks VMs based on the QEMU hypervisor now can use option for -- 2.11.0
Re: Hyper-V CPU details
On Thu, Oct 22, 2020 at 11:01 PM Matt Coleman wrote: > > Hello, > > I'm implementing domainGetVcpus and could use some guidance on what > value to use for virVcpuInfo->cpu. > > Hyper-V does not allow the user to pin vCPUs to host CPUs and doesn't > allow the user to see which host CPU a vCPU is currently running on. > Since it's a type 1 hypervisor, none of its scheduling data is > available to the Windows userspace: there aren't any processes or > threads that correspond to vCPUs that I could query the OS scheduler > about. > > My code currently sets it to -1, which produces the following `virsh > vcpuinfo` output for a running VM with two cores: > > VCPU: 0 > CPU:-1 > State: running > CPU time: 1684.5s > CPU Affinity: > > VCPU: 1 > CPU:-1 > State: running > CPU time: 1346.0s > CPU Affinity: > > However, this doesn't match the comment in _virVcpuInfo's declaration, > which says that -1 signifies an offline CPU: > > https://gitlab.com/libvirt/libvirt/-/blob/v6.8.0/include/libvirt/libvirt-domain.h#L1918 > > Should I stick with -1? Or, should I introduce -2 as a value that > indicates that the hypervisor doesn't provide that information? Or, is > there some better way to handle this that I'm not aware of? > I would suggest introducing a new value. The problem with overloading values is that if there's other code expecting that to mean something, it gets confusing really quickly. -- 真実はいつも一つ！/ Always, there's only one truth!
Re: [PATCH 0/6] Hyper-V code cleanup
On Thu, Oct 22, 2020 at 12:38 PM Matt Coleman wrote: > > Here's a draft GitLab MR if you'd prefer to review the changes there: > https://gitlab.com/iammattcoleman/libvirt/-/merge_requests/3/diffs > > Matt Coleman (6): > hyperv: reformat WQL query strings > hyperv: remove duplicate function hypervGetVSSDFromUUID() > hyperv: remove duplicate function hypervGetMemSDByVSSDInstanceId() > hyperv: remove unneeded braces in hypervDomainGetInfo() and > hypervDomainGetXMLDesc() > hyperv: reduce duplicate code for Msvm_ComputerSystem lookups > hyperv: do not overwrite errors from hypervInvokeMethod() > > src/hyperv/hyperv_driver.c | 169 + > src/hyperv/hyperv_wmi.c| 57 +++-- > src/hyperv/hyperv_wmi.h| 4 + > 3 files changed, 75 insertions(+), 155 deletions(-) > > -- > 2.27.0 > > Reviewed-by: Neal Gompa -- 真実はいつも一つ！/ Always, there's only one truth!
Re: [PATCH 0/4] hyperv: Deduplicate and reformat
On Wed, Oct 21, 2020 at 11:45 AM Michal Privoznik wrote: > > The more I look into the code the more things to fix I find. Well, here > are some fixes. > > Michal Prívozník (4): > hyperv: Don't overwrite errors from hypervCreateInvokeParamsList() > hyperv: Use hypervRequestStateChange() in hypervDomainSuspend() > hyperv: Use two empty lines between functions > hyperv: Reformat > > src/hyperv/hyperv_driver.c | 186 +--- > src/hyperv/hyperv_util.c | 1 - > src/hyperv/hyperv_wmi.c| 288 +++-- > src/hyperv/hyperv_wmi.h| 15 +- > 4 files changed, 229 insertions(+), 261 deletions(-) > > -- > 2.26.2 > Series LGTM. Reviewed-by: Neal Gompa But... uhh... there's also this series from Matt Coleman, which I just reviewed: https://www.redhat.com/archives/libvir-list/2020-October/msg01282.html Can we put the two together somehow? -- 真実はいつも一つ！/ Always, there's only one truth!
Re: [PATCH 0/4] hyperv: Deduplicate and reformat
On Fri, Oct 23, 2020 at 10:53 AM Neal Gompa wrote: > > On Wed, Oct 21, 2020 at 11:45 AM Michal Privoznik wrote: > > > > The more I look into the code the more things to fix I find. Well, here > > are some fixes. > > > > Michal Prívozník (4): > > hyperv: Don't overwrite errors from hypervCreateInvokeParamsList() > > hyperv: Use hypervRequestStateChange() in hypervDomainSuspend() > > hyperv: Use two empty lines between functions > > hyperv: Reformat > > > > src/hyperv/hyperv_driver.c | 186 +--- > > src/hyperv/hyperv_util.c | 1 - > > src/hyperv/hyperv_wmi.c| 288 +++-- > > src/hyperv/hyperv_wmi.h| 15 +- > > 4 files changed, 229 insertions(+), 261 deletions(-) > > > > -- > > 2.26.2 > > > > Series LGTM. > > Reviewed-by: Neal Gompa > > But... uhh... there's also this series from Matt Coleman, which I just > reviewed: > https://www.redhat.com/archives/libvir-list/2020-October/msg01282.html > > Can we put the two together somehow? > Ignore me, apparently this was seen by him already (his review was in spam...) -- 真実はいつも一つ！/ Always, there's only one truth!
Re: [libvirt PATCH] qemu: fix potential resource leak
On 10/23/20 1:28 AM, Peter Krempa wrote:
On Thu, Oct 22, 2020 at 23:20:20 -0400, Laine Stump wrote:
On 10/22/20 3:01 AM, Peter Krempa wrote:
On Wed, Oct 21, 2020 at 22:31:09 -0400, Laine Stump wrote:
On 10/21/20 5:50 PM, Jonathon Jongsma wrote:
Coverity reported a potential resource leak. While it's probably not
a real-world scenario, the code could technically jump to cleanup
between the time that vdpafd is opened and when it is used. Ensure that
it gets cleaned up in that case.
Signed-off-by: Jonathon Jongsma
---
src/qemu/qemu_command.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5c4e37bd9e..cbe7a6e331 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8135,6 +8135,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
addfdarg = g_strdup_printf("%s,opaque=%s", fdset,
net->data.vdpa.devicepath);
virCommandAddArgList(cmd, "-add-fd", addfdarg, NULL);
+vdpafd = -1;
}
if (chardev)
@@ -8204,6 +8205,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
VIR_FREE(tapfdName);
VIR_FREE(vhostfd);
VIR_FREE(tapfd);
+if (vdpafd >= 0)
+VIR_FORCE_CLOSE(vdpafd);
VIR_FORCE_CLOSE() ==>virForceCloseHelper() ==> virFileClose()
and virFileClose() is a NOP if fd < 0, so this doesn't need the conditional.
I *was* going to say "With that change,
Reviewed-by: Laine Stump
"
but this got me looking at the code of the entire function rather than just
the diffs in the patch, and I've got a question - is there any reason that
you only ope n the vdpa device inside the switch, and save everything else
related to it until later (at the "if (vdpafd)")? You could then device
I'd like to point out that opening anything in the command line
formatters is IMO bad practice.
Well... I agree that it is an ugly design, but that's pretty much what's in
place for almost everything.
Sure, but it shouldn't be used as an argument to not use a better
approach.
The command line formatter should format
the commandline and nothing more.
It would be nice if that was the case, but it already isn't. :-/
No. Please don't put it this way. My message is that while the old code
isn't compliant, it shouldn't be an excuse to make it worse!
Yes, I can see how my statement might be misunderstood as "the battle is
already lost! Give up!!", which wasn't my intent.
In this instace the code is commited. We don't need to change it,
^^ that was more my intent, and it seems we agree.
but
any further change should be encouraged to use the better approach.
This is visible by the necessity to
have a lot of mock override functions which prevent the command line
formatter from touching resources on the host in the first place.
Better approach is to open resources in 'qemuProcessPrepareHost' and
store them in private data for command line formatting. Fake data for
tests are added in 'testCompareXMLToArgvCreateArgs'.
That's nice in the fact that it eliminates the need for mock overrides
(would be even *nicer* if that function had even a single line of
documentation included that described its purpose, and what code in the qemu
driver it should be mimicking, amirite?).
I agree, documentation is lacking in many parts. This is the
not-so-obvious techincal debt.
But it's bad because the code in qemuProcessPrepareHost() won't be tested
(can't be tested if there are no mocks for the system functions it calls).
IMO this statement is misrepresenting what's happening. Sure
qemuProcessPrepareHost can't be tested by unit testing. It's replaced by
another function which fakes inputs. But that is EXACTLY what the mock
functions preloaded into our tests are doing!
With qemuProcessPrepareHost() you at least have one place and one
function where all the code is aggregated
Where all the code *to prepare/setup devices* is aggregated. But in the
process of aggregating that code, you've *split* the code for each
individual device into multiple places with possibly no overt connection
between them. Which of those is better depends on what you're doing. If
you're adding a new feature, it means there is one more place you have
to seek out and add something to. If there is something in each of those
places that causes a build error when its omitted from the additions
(e.g. a switch statement that needs a new case) then everything is
great, but if it's just something that people are expected to find
themselves by following the bread crumbs of a similar existing feature,
then it becomes more trouble.
Keep in mind, I'm not saying that this is worse than the old way, just
that it brings in its own new set of issues.
rather than spreading it
trhough the command line generator and it being very hard to audit
afterwards.
Basically you're trading the extra work of mocking system-level functions
for the ex
Re: [PATCH] pci: Refuse to hotplug PCI Devices when the Guest OS is not ready
On Fri, 23 Oct 2020 11:54:40 -0400 "Michael S. Tsirkin" wrote: > On Fri, Oct 23, 2020 at 09:47:14AM +0300, Marcel Apfelbaum wrote: > > Hi David, > > > > On Fri, Oct 23, 2020 at 6:49 AM David Gibson wrote: > > > > On Thu, 22 Oct 2020 11:01:04 -0400 > > "Michael S. Tsirkin" wrote: > > > > > On Thu, Oct 22, 2020 at 05:50:51PM +0300, Marcel Apfelbaum wrote: > > > [...] > > > > > > Right. After detecting just failing unconditionally it a bit too > > > simplistic IMHO. > > > > There's also another factor here, which I thought I'd mentioned > > already, but looks like I didn't: I think we're still missing some > > details in what's going on. > > > > The premise for this patch is that plugging while the indicator is in > > transition state is allowed to fail in any way on the guest side. I > > don't think that's a reasonable interpretation, because it's unworkable > > for physical hotplug. If the indicator starts blinking while you're > > in > > the middle of shoving a card in, you'd be in trouble. > > > > So, what I'm assuming here is that while "don't plug while blinking" is > > the instruction for the operator to obey as best they can, on the guest > > side the rule has to be "start blinking, wait a while and by the time > > you leave blinking state again, you can be confident any plugs or > > unplugs have completed". Obviously still racy in the strict computer > > science sense, but about the best you can do with slow humans in the > > mix. > > > > So, qemu should of course endeavour to follow that rule as though it > > was a human operator on a physical machine and not plug when the > > indicator is blinking. *But* the qemu plug will in practice be fast > > enough that if we're hitting real problems here, it suggests the guest > > is still doing something wrong. > > > > > > I personally think there is a little bit of over-engineering here. > > Let's start with the spec: > > > >   Power Indicator Blinking > >   A blinking Power Indicator indicates that the slot is powering up > > or > > powering down and that > >   insertion or removal of the adapter is not permitted. > > > > What exactly is an interpretation here? > > As you stated, the races are theoretical, the whole point of the indicator > > is to let the operator know he can't plug the device just yet. > > > > I understand it would be more user friendly if the QEMU would wait > > internally > > for the > > blinking to end, but the whole point of the indicator is to let the > > operator > > (human or machine) > > know they can't plug the device at a specific time. > > Should QEMU take the responsibility of the operator? Is it even correct? > > > > Even if we would want such a feature, how is it related to this patch? > > The patch simply refuses to start a hotplug operation when it knows it will > > not > > succeed. > >  > > Another way that would make sense to me would be is a new QEMU > > interface other > > than > > "add_device", let's say "adding_device_allowed", that would return true if > > the > > hotplug is allowed > > at this point of time. (I am aware of the theoretical races) > > Rather than adding_device_allowed, something like "query slot" > might be helpful for debugging. That would help user figure out > e.g. why isn't device visible without any races. Would be new command useful tough? What we end up is broken guest (if I read commit message right) and a user who has no idea if device_add was successful or not. So what user should do in this case - wait till it explodes? - can user remove it or it would be stuck there forever? - poll slot before hotplug, manually? (if this is the case then failing device_add cleanly doesn't sound bad, it looks similar to another error we have "/* Check if hot-plug is disabled on the slot */" in pcie_cap_slot_pre_plug_cb) CCing libvirt, as it concerns not only QEMU. > > > The above will at least mimic the mechanics of the pyhs world. The > > operator > > looks at the indicator, > > the management software checks if adding the device is allowed. > > Since it is a corner case I would prefer the device_add to fail rather than > > introducing a new interface, > > but that's just me. > > > > Thanks, > > Marcel > > > > I think we want QEMU management interface to be reasonably > abstract and agnostic if possible. Pushing knowledge of hardware > detail to management will just lead to pain IMHO. > We supported device_add which practically never fails for years, For CPUs and RAM, device_add can fail so maybe management is also prepared to handle errors on PCI hotplug path. > at this point it's easier to keep supporting it than > change all users ... > > > > > > -- > > David Gibson > > Principal Software Engineer, Virtualization, Red Hat > > > >
[PATCH 09/10] conf: node_device: cleanup virNodeDevCapCCWParseXML
Make use of g_autofree
Signed-off-by: Boris Fiuczynski
Reviewed-by: Bjoern Walk
---
src/conf/node_device_conf.c | 27 +++
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index ebabf20b67..39950565b5 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -839,57 +839,52 @@ virNodeDevCapCCWParseXML(xmlXPathContextPtr ctxt,
virNodeDevCapCCWPtr ccw_dev)
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-int ret = -1;
-char *cssid = NULL, *ssid = NULL, *devno = NULL;
+g_autofree char *cssid = NULL;
+g_autofree char *ssid = NULL;
+g_autofree char *devno = NULL;
ctxt->node = node;
- if (!(cssid = virXPathString("string(./cssid[1])", ctxt))) {
+if (!(cssid = virXPathString("string(./cssid[1])", ctxt))) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("missing cssid value for '%s'"), def->name);
-goto out;
+return -1;
}
if (virStrToLong_uip(cssid, NULL, 0, &ccw_dev->cssid) < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("invalid cssid value '%s' for '%s'"),
cssid, def->name);
-goto out;
+return -1;
}
if (!(ssid = virXPathString("string(./ssid[1])", ctxt))) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("missing ssid value for '%s'"), def->name);
-goto out;
+return -1;
}
if (virStrToLong_uip(ssid, NULL, 0, &ccw_dev->ssid) < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("invalid ssid value '%s' for '%s'"),
cssid, def->name);
-goto out;
+return -1;
}
if (!(devno = virXPathString("string(./devno[1])", ctxt))) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("missing devno value for '%s'"), def->name);
-goto out;
+return -1;
}
if (virStrToLong_uip(devno, NULL, 16, &ccw_dev->devno) < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("invalid devno value '%s' for '%s'"),
devno, def->name);
-goto out;
+return -1;
}
-ret = 0;
-
- out:
-VIR_FREE(cssid);
-VIR_FREE(ssid);
-VIR_FREE(devno);
-return ret;
+return 0;
}
--
2.25.1
[PATCH 04/10] conf: node_devive: refactor GetPCIMdevTypesCaps into GetMdevTypeCapes
Extracting PCI from virNodeDeviceGetPCIMdevTypesCaps creating
virNodeDeviceGetMdevTypesCaps to make later reuse possible.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Bjoern Walk
---
src/conf/node_device_conf.c | 31 +++
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index af8edded3c..811d102208 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2570,26 +2570,25 @@
virNodeDeviceGetPCIIOMMUGroupCaps(virNodeDevCapPCIDevPtr pci_dev)
static int
-virNodeDeviceGetPCIMdevTypesCaps(const char *sysfspath,
- virNodeDevCapPCIDevPtr pci_dev)
+virNodeDeviceGetMdevTypesCaps(const char *sysfspath,
+ virMediatedDeviceTypePtr **mdev_types,
+ size_t *nmdev_types)
{
virMediatedDeviceTypePtr *types = NULL;
size_t ntypes = 0;
size_t i;
/* this could be a refresh, so clear out the old data */
-for (i = 0; i < pci_dev->nmdev_types; i++)
- virMediatedDeviceTypeFree(pci_dev->mdev_types[i]);
-VIR_FREE(pci_dev->mdev_types);
-pci_dev->nmdev_types = 0;
-pci_dev->flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_MDEV;
+for (i = 0; i < *nmdev_types; i++)
+ virMediatedDeviceTypeFree(*mdev_types[i]);
+VIR_FREE(*mdev_types);
+*nmdev_types = 0;
if (virMediatedDeviceGetMdevTypes(sysfspath, &types, &ntypes) < 0)
return -1;
-pci_dev->mdev_types = g_steal_pointer(&types);
-pci_dev->nmdev_types = ntypes;
-pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV;
+*mdev_types = g_steal_pointer(&types);
+*nmdev_types = ntypes;
return 0;
}
@@ -2606,9 +2605,17 @@ virNodeDeviceGetPCIDynamicCaps(const char *sysfsPath,
virNodeDevCapPCIDevPtr pci_dev)
{
if (virNodeDeviceGetPCISRIOVCaps(sysfsPath, pci_dev) < 0 ||
-virNodeDeviceGetPCIIOMMUGroupCaps(pci_dev) < 0 ||
-virNodeDeviceGetPCIMdevTypesCaps(sysfsPath, pci_dev) < 0)
+virNodeDeviceGetPCIIOMMUGroupCaps(pci_dev) < 0)
+return -1;
+
+pci_dev->flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_MDEV;
+if (virNodeDeviceGetMdevTypesCaps(sysfsPath,
+ &pci_dev->mdev_types,
+ &pci_dev->nmdev_types) < 0)
return -1;
+if (pci_dev->nmdev_types > 0)
+pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV;
+
return 0;
}
--
2.25.1
[PATCH 08/10] schema: refactor mdev_types out of PCI nodedev schema
Refactor mdev_types into standalone define for later reuse. Signed-off-by: Boris Fiuczynski Reviewed-by: Bjoern Walk --- docs/schemas/nodedev.rng | 48 ++-- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 166e278cf8..9548412999 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -215,27 +215,7 @@ - - - mdev_types - - - - - - - - - - - vfio-pci - - - - - - - + @@ -696,4 +676,30 @@ + + + +mdev_types + + + + + + + + + + + + vfio-pci + + + + + + + + + + -- 2.25.1
[PATCH 02/10] util: refactor mdev_types method from PCI to mdev
Extract virPCIGetMdevTypes from PCI as virMediatedDeviceGetMdevTypes
into mdev for later reuse.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Bjoern Walk
---
src/conf/node_device_conf.c | 2 +-
src/libvirt_private.syms| 2 +-
src/util/virmdev.c | 65 +
src/util/virmdev.h | 4 +++
src/util/virpci.c | 60 --
src/util/virpci.h | 3 --
6 files changed, 71 insertions(+), 65 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 3fbe9338ee..db1258436a 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2584,7 +2584,7 @@ virNodeDeviceGetPCIMdevTypesCaps(const char *sysfspath,
pci_dev->nmdev_types = 0;
pci_dev->flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_MDEV;
-rc = virPCIGetMdevTypes(sysfspath, &types);
+rc = virMediatedDeviceGetMdevTypes(sysfspath, &types);
if (rc <= 0)
return rc;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 95e50835ad..9029ea4fa2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2509,6 +2509,7 @@ virMediatedDeviceAttrNew;
virMediatedDeviceFree;
virMediatedDeviceGetIOMMUGroupDev;
virMediatedDeviceGetIOMMUGroupNum;
+virMediatedDeviceGetMdevTypes;
virMediatedDeviceGetSysfsPath;
virMediatedDeviceGetUsedBy;
virMediatedDeviceIsUsed;
@@ -2845,7 +2846,6 @@ virPCIELinkSpeedTypeFromString;
virPCIELinkSpeedTypeToString;
virPCIGetDeviceAddressFromSysfsLink;
virPCIGetHeaderType;
-virPCIGetMdevTypes;
virPCIGetNetName;
virPCIGetPhysicalFunction;
virPCIGetVirtualFunctionIndex;
diff --git a/src/util/virmdev.c b/src/util/virmdev.c
index 31994631ed..80f5f2a767 100644
--- a/src/util/virmdev.c
+++ b/src/util/virmdev.c
@@ -522,3 +522,68 @@ void virMediatedDeviceAttrFree(virMediatedDeviceAttrPtr
attr)
g_free(attr->value);
g_free(attr);
}
+
+
+#ifdef __linux__
+
+ssize_t
+virMediatedDeviceGetMdevTypes(const char *sysfspath,
+ virMediatedDeviceTypePtr **types)
+{
+ssize_t ret = -1;
+int dirret = -1;
+DIR *dir = NULL;
+struct dirent *entry;
+g_autofree char *types_path = NULL;
+g_autoptr(virMediatedDeviceType) mdev_type = NULL;
+virMediatedDeviceTypePtr *mdev_types = NULL;
+size_t ntypes = 0;
+size_t i;
+
+types_path = g_strdup_printf("%s/mdev_supported_types", sysfspath);
+
+if ((dirret = virDirOpenIfExists(&dir, types_path)) < 0)
+goto cleanup;
+
+if (dirret == 0) {
+ret = 0;
+goto cleanup;
+}
+
+while ((dirret = virDirRead(dir, &entry, types_path)) > 0) {
+g_autofree char *tmppath = NULL;
+/* append the type id to the path and read the attributes from there */
+tmppath = g_strdup_printf("%s/%s", types_path, entry->d_name);
+
+if (virMediatedDeviceTypeReadAttrs(tmppath, &mdev_type) < 0)
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(mdev_types, ntypes, mdev_type) < 0)
+goto cleanup;
+}
+
+if (dirret < 0)
+goto cleanup;
+
+*types = g_steal_pointer(&mdev_types);
+ret = ntypes;
+ntypes = 0;
+ cleanup:
+for (i = 0; i < ntypes; i++)
+virMediatedDeviceTypeFree(mdev_types[i]);
+VIR_FREE(mdev_types);
+VIR_DIR_CLOSE(dir);
+return ret;
+}
+
+#else
+
+ssize_t
+virMediatedDeviceGetMdevTypes(const char *sysfspath G_GNUC_UNUSED,
+ virMediatedDeviceTypePtr **types G_GNUC_UNUSED)
+{
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
+return -1;
+}
+
+#endif /* __linux__ */
diff --git a/src/util/virmdev.h b/src/util/virmdev.h
index eb167ccb48..846e1662e7 100644
--- a/src/util/virmdev.h
+++ b/src/util/virmdev.h
@@ -149,5 +149,9 @@ int
virMediatedDeviceTypeReadAttrs(const char *sysfspath,
virMediatedDeviceTypePtr *type);
+ssize_t
+virMediatedDeviceGetMdevTypes(const char *sysfspath,
+ virMediatedDeviceTypePtr **types);
+
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virMediatedDevice, virMediatedDeviceFree);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virMediatedDeviceType,
virMediatedDeviceTypeFree);
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 1f679a7b45..2086a4270b 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2528,57 +2528,6 @@ virPCIGetVirtualFunctionInfo(const char
*vf_sysfs_device_path,
return 0;
}
-
-ssize_t
-virPCIGetMdevTypes(const char *sysfspath,
- virMediatedDeviceTypePtr **types)
-{
-ssize_t ret = -1;
-int dirret = -1;
-DIR *dir = NULL;
-struct dirent *entry;
-g_autofree char *types_path = NULL;
-g_autoptr(virMediatedDeviceType) mdev_type = NULL;
-virMediatedDeviceTypePtr *mdev_types = NULL;
-size_t ntypes = 0;
-size_t i;
-
-types_path = g_strdup_printf("%s/mdev_supported_types", sysfspath);
-
-if ((dirret = virDirOpenIfExists(&dir, types_path)) < 0)
-
[PATCH 10/10] node_device: detecting mdev_types capability on CSS devices
Add detection of mdev_types capability to channel subsystem devices.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Bjoern Walk
---
docs/drvnodedev.html.in | 5 +-
docs/formatnode.html.in | 39
docs/schemas/nodedev.rng | 4 +
src/conf/node_device_conf.c | 92 ++-
src/conf/node_device_conf.h | 11 +++
src/conf/virnodedeviceobj.c | 7 +-
src/libvirt_private.syms | 1 +
src/node_device/node_device_udev.c| 3 +
.../css_0_0_fffe_mdev_types.xml | 17
tests/nodedevxml2xmltest.c| 1 +
10 files changed, 175 insertions(+), 5 deletions(-)
create mode 100644 tests/nodedevschemadata/css_0_0_fffe_mdev_types.xml
diff --git a/docs/drvnodedev.html.in b/docs/drvnodedev.html.in
index 0823c1888d..d5191d6d93 100644
--- a/docs/drvnodedev.html.in
+++ b/docs/drvnodedev.html.in
@@ -139,12 +139,13 @@
MDEV capability
- A PCI device capable of creating mediated devices will include a nested
+ A device capable of creating mediated devices will include a nested
capability mdev_types which enumerates all supported mdev
types on the physical device, along with the type attributes available
through sysfs. A detailed description of the XML format for the
mdev_types capability can be found
- here.
+ here for PCI or
+ here for CSS.
The following example shows how we might represent an NVIDIA GPU device
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index 594427468b..7f3c71941d 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -430,6 +430,45 @@
The subchannel-set identifier.
devno
The device number.
+ capability
+
+This optional element can occur multiple times. If it
+exists, it has a mandatory type attribute
+which will be set to:
+
+ mdev_types
+
+This device is capable of creating mediated devices, and
+the capability will contain a list of type
+elements, which list all mdev types supported on the
+physical device. Since 6.9.0
+Each type element has a single id
+attribute that holds an official vendor-supplied identifier
+for the type. It supports the following sub-elements:
+
+ name
+
+The name element holds a vendor-supplied
+code name for the given mediated device type. This is
+an optional element.
+
+ deviceAPI
+
+The value of this element describes how an instance of
+the given type will be presented to the guest by the
+VFIO framework.
+
+ availableInstances
+
+This element reports the current state of resource
+allocation. In other words, how many instances of the
+given type can still be successfully created on the
+physical device.
+
+
+
+
+
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 9548412999..d3248e90a9 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -653,6 +653,9 @@
+
+
+
@@ -692,6 +695,7 @@
vfio-pci
+ vfio-ccw
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 39950565b5..75c033b0e1 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -544,6 +544,10 @@ virNodeDeviceCapCCWDefFormat(virBufferPtr buf,
data->ccw_dev.ssid);
virBufferAsprintf(buf, "0x%04x\n",
data->ccw_dev.devno);
+if (data->ccw_dev.flags & VIR_NODE_DEV_CAP_FLAG_CSS_MDEV)
+virNodeDeviceCapMdevTypesFormat(buf,
+data->ccw_dev.mdev_types,
+data->ccw_dev.nmdev_types);
}
@@ -832,6 +836,33 @@ virNodeDevCapMdevTypesParseXML(xmlXPathContextPtr ctxt,
}
+static int
+virNodeDevCSSCapabilityParseXML(xmlXPathContextPtr ctxt,
+xmlNodePtr node,
+virNodeDevCapCCWPtr ccw_dev)
+{
+g_autofree char *type = virXMLPropString(node, "type");
[PATCH 01/10] conf: node_device: fix mdev_types format and XML parsing code to match schema
The nodedev schema defines that a mdev_types capability must have
one or more type elements. The XML parsing and the format allows to
accept and to write mdev_types capability without any type element.
This patches fixes this.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Bjoern Walk
---
src/conf/node_device_conf.c | 40 ++---
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 4adfdef572..3fbe9338ee 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -276,25 +276,27 @@ virNodeDeviceCapPCIDefFormat(virBufferPtr buf,
virPCIHeaderTypeToString(data->pci_dev.hdrType));
}
if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) {
-virBufferAddLit(buf, "\n");
-virBufferAdjustIndent(buf, 2);
-for (i = 0; i < data->pci_dev.nmdev_types; i++) {
-virMediatedDeviceTypePtr type = data->pci_dev.mdev_types[i];
-virBufferEscapeString(buf, "\n", type->id);
+if (data->pci_dev.nmdev_types > 0) {
+virBufferAddLit(buf, "\n");
virBufferAdjustIndent(buf, 2);
-if (type->name)
-virBufferEscapeString(buf, "%s\n",
- type->name);
-virBufferEscapeString(buf, "%s\n",
- type->device_api);
-virBufferAsprintf(buf,
- "%u\n",
- type->available_instances);
+for (i = 0; i < data->pci_dev.nmdev_types; i++) {
+virMediatedDeviceTypePtr type = data->pci_dev.mdev_types[i];
+virBufferEscapeString(buf, "\n", type->id);
+virBufferAdjustIndent(buf, 2);
+if (type->name)
+virBufferEscapeString(buf, "%s\n",
+ type->name);
+virBufferEscapeString(buf, "%s\n",
+ type->device_api);
+virBufferAsprintf(buf,
+
"%u\n",
+ type->available_instances);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+}
virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
+virBufferAddLit(buf, "\n");
}
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
}
if (data->pci_dev.nIommuGroupDevices) {
virBufferAsprintf(buf, "\n",
@@ -1520,6 +1522,12 @@ virNodeDevPCICapMdevTypesParseXML(xmlXPathContextPtr
ctxt,
if ((nmdev_types = virXPathNodeSet("./type", ctxt, &nodes)) < 0)
goto cleanup;
+if (nmdev_types == 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing element in element"));
+goto cleanup;
+}
+
orignode = ctxt->node;
for (i = 0; i < nmdev_types; i++) {
ctxt->node = nodes[i];
--
2.25.1
[PATCH 05/10] conf: node_device: refactor capability mdev_types formating
Extract the XML formating for mdev_types from PCI capability into
a generic standalone method for later reuse.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Bjoern Walk
---
src/conf/node_device_conf.c | 55 +++--
1 file changed, 34 insertions(+), 21 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 811d102208..dce04ad4f0 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -206,6 +206,37 @@ virNodeDeviceCapSystemDefFormat(virBufferPtr buf,
}
+static void
+virNodeDeviceCapMdevTypesFormat(virBufferPtr buf,
+virMediatedDeviceTypePtr *mdev_types,
+const size_t nmdev_types)
+{
+size_t i;
+
+if (nmdev_types > 0) {
+virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(buf, 2);
+for (i = 0; i < nmdev_types; i++) {
+virMediatedDeviceTypePtr type = mdev_types[i];
+virBufferEscapeString(buf, "\n", type->id);
+virBufferAdjustIndent(buf, 2);
+if (type->name)
+virBufferEscapeString(buf, "%s\n",
+ type->name);
+virBufferEscapeString(buf, "%s\n",
+ type->device_api);
+virBufferAsprintf(buf,
+ "%u\n",
+ type->available_instances);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+}
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+}
+}
+
+
static void
virNodeDeviceCapPCIDefFormat(virBufferPtr buf,
const virNodeDevCapData *data)
@@ -276,27 +307,9 @@ virNodeDeviceCapPCIDefFormat(virBufferPtr buf,
virPCIHeaderTypeToString(data->pci_dev.hdrType));
}
if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) {
-if (data->pci_dev.nmdev_types > 0) {
-virBufferAddLit(buf, "\n");
-virBufferAdjustIndent(buf, 2);
-for (i = 0; i < data->pci_dev.nmdev_types; i++) {
-virMediatedDeviceTypePtr type = data->pci_dev.mdev_types[i];
-virBufferEscapeString(buf, "\n", type->id);
-virBufferAdjustIndent(buf, 2);
-if (type->name)
-virBufferEscapeString(buf, "%s\n",
- type->name);
-virBufferEscapeString(buf, "%s\n",
- type->device_api);
-virBufferAsprintf(buf,
-
"%u\n",
- type->available_instances);
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
-}
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
-}
+virNodeDeviceCapMdevTypesFormat(buf,
+data->pci_dev.mdev_types,
+data->pci_dev.nmdev_types);
}
if (data->pci_dev.nIommuGroupDevices) {
virBufferAsprintf(buf, "\n",
--
2.25.1
[PATCH 03/10] util: refactor mdev_types methods return code usage
Remove mix of array length and error code in the return code.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Bjoern Walk
---
src/conf/node_device_conf.c | 10 --
src/util/virmdev.c | 14 --
src/util/virmdev.h | 3 ++-
3 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index db1258436a..af8edded3c 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2574,7 +2574,7 @@ virNodeDeviceGetPCIMdevTypesCaps(const char *sysfspath,
virNodeDevCapPCIDevPtr pci_dev)
{
virMediatedDeviceTypePtr *types = NULL;
-int rc = 0;
+size_t ntypes = 0;
size_t i;
/* this could be a refresh, so clear out the old data */
@@ -2584,13 +2584,11 @@ virNodeDeviceGetPCIMdevTypesCaps(const char *sysfspath,
pci_dev->nmdev_types = 0;
pci_dev->flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_MDEV;
-rc = virMediatedDeviceGetMdevTypes(sysfspath, &types);
-
-if (rc <= 0)
-return rc;
+if (virMediatedDeviceGetMdevTypes(sysfspath, &types, &ntypes) < 0)
+return -1;
pci_dev->mdev_types = g_steal_pointer(&types);
-pci_dev->nmdev_types = rc;
+pci_dev->nmdev_types = ntypes;
pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV;
return 0;
diff --git a/src/util/virmdev.c b/src/util/virmdev.c
index 80f5f2a767..b02005bd1a 100644
--- a/src/util/virmdev.c
+++ b/src/util/virmdev.c
@@ -528,7 +528,8 @@ void virMediatedDeviceAttrFree(virMediatedDeviceAttrPtr
attr)
ssize_t
virMediatedDeviceGetMdevTypes(const char *sysfspath,
- virMediatedDeviceTypePtr **types)
+ virMediatedDeviceTypePtr **types,
+ size_t *ntypes)
{
ssize_t ret = -1;
int dirret = -1;
@@ -537,7 +538,7 @@ virMediatedDeviceGetMdevTypes(const char *sysfspath,
g_autofree char *types_path = NULL;
g_autoptr(virMediatedDeviceType) mdev_type = NULL;
virMediatedDeviceTypePtr *mdev_types = NULL;
-size_t ntypes = 0;
+size_t nmdev_types = 0;
size_t i;
types_path = g_strdup_printf("%s/mdev_supported_types", sysfspath);
@@ -558,7 +559,7 @@ virMediatedDeviceGetMdevTypes(const char *sysfspath,
if (virMediatedDeviceTypeReadAttrs(tmppath, &mdev_type) < 0)
goto cleanup;
-if (VIR_APPEND_ELEMENT(mdev_types, ntypes, mdev_type) < 0)
+if (VIR_APPEND_ELEMENT(mdev_types, nmdev_types, mdev_type) < 0)
goto cleanup;
}
@@ -566,10 +567,11 @@ virMediatedDeviceGetMdevTypes(const char *sysfspath,
goto cleanup;
*types = g_steal_pointer(&mdev_types);
-ret = ntypes;
-ntypes = 0;
+*ntypes = nmdev_types;
+nmdev_types = 0;
+ret = 0;
cleanup:
-for (i = 0; i < ntypes; i++)
+for (i = 0; i < nmdev_types; i++)
virMediatedDeviceTypeFree(mdev_types[i]);
VIR_FREE(mdev_types);
VIR_DIR_CLOSE(dir);
diff --git a/src/util/virmdev.h b/src/util/virmdev.h
index 846e1662e7..b6563a94fc 100644
--- a/src/util/virmdev.h
+++ b/src/util/virmdev.h
@@ -151,7 +151,8 @@ virMediatedDeviceTypeReadAttrs(const char *sysfspath,
ssize_t
virMediatedDeviceGetMdevTypes(const char *sysfspath,
- virMediatedDeviceTypePtr **types);
+ virMediatedDeviceTypePtr **types,
+ size_t *ntypes);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virMediatedDevice, virMediatedDeviceFree);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virMediatedDeviceType,
virMediatedDeviceTypeFree);
--
2.25.1
[PATCH 06/10] conf: node_device: refactor mdev_types XML parsing
Extract PCI code from virNodeDevPCICapMdevTypesParseXML to make
method virNodeDevCapMdevTypesParseXML generic for later reuse.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Bjoern Walk
---
src/conf/node_device_conf.c | 141 ++--
1 file changed, 72 insertions(+), 69 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index dce04ad4f0..6fd5b1b038 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -757,6 +757,72 @@ virNodeDevCapDRMParseXML(xmlXPathContextPtr ctxt,
}
+static int
+virNodeDevCapMdevTypesParseXML(xmlXPathContextPtr ctxt,
+ virMediatedDeviceTypePtr **mdev_types,
+ size_t *nmdev_types)
+{
+int ret = -1;
+xmlNodePtr orignode = NULL;
+xmlNodePtr *nodes = NULL;
+int ntypes = -1;
+virMediatedDeviceTypePtr type = NULL;
+size_t i;
+
+if ((ntypes = virXPathNodeSet("./type", ctxt, &nodes)) < 0)
+goto cleanup;
+
+if (nmdev_types == 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing element in element"));
+goto cleanup;
+}
+
+orignode = ctxt->node;
+for (i = 0; i < ntypes; i++) {
+ctxt->node = nodes[i];
+
+type = g_new0(virMediatedDeviceType, 1);
+
+if (!(type->id = virXPathString("string(./@id[1])", ctxt))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing 'id' attribute for mediated device's "
+ " element"));
+goto cleanup;
+}
+
+if (!(type->device_api = virXPathString("string(./deviceAPI[1])",
ctxt))) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("missing device API for mediated device type
'%s'"),
+ type->id);
+goto cleanup;
+}
+
+if (virXPathUInt("number(./availableInstances)", ctxt,
+ &type->available_instances) < 0) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("missing number of available instances for "
+ "mediated device type '%s'"),
+ type->id);
+goto cleanup;
+}
+
+type->name = virXPathString("string(./name)", ctxt);
+
+if (VIR_APPEND_ELEMENT(*mdev_types,
+ *nmdev_types, type) < 0)
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(nodes);
+virMediatedDeviceTypeFree(type);
+ctxt->node = orignode;
+return ret;
+}
+
+
static int
virNodeDevCapCCWParseXML(xmlXPathContextPtr ctxt,
virNodeDeviceDefPtr def,
@@ -1521,72 +1587,6 @@ virNodeDevPCICapSRIOVVirtualParseXML(xmlXPathContextPtr
ctxt,
}
-static int
-virNodeDevPCICapMdevTypesParseXML(xmlXPathContextPtr ctxt,
- virNodeDevCapPCIDevPtr pci_dev)
-{
-int ret = -1;
-xmlNodePtr orignode = NULL;
-xmlNodePtr *nodes = NULL;
-int nmdev_types = -1;
-virMediatedDeviceTypePtr type = NULL;
-size_t i;
-
-if ((nmdev_types = virXPathNodeSet("./type", ctxt, &nodes)) < 0)
-goto cleanup;
-
-if (nmdev_types == 0) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing element in element"));
-goto cleanup;
-}
-
-orignode = ctxt->node;
-for (i = 0; i < nmdev_types; i++) {
-ctxt->node = nodes[i];
-
-type = g_new0(virMediatedDeviceType, 1);
-
-if (!(type->id = virXPathString("string(./@id[1])", ctxt))) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing 'id' attribute for mediated device's "
- " element"));
-goto cleanup;
-}
-
-if (!(type->device_api = virXPathString("string(./deviceAPI[1])",
ctxt))) {
-virReportError(VIR_ERR_XML_ERROR,
- _("missing device API for mediated device type
'%s'"),
- type->id);
-goto cleanup;
-}
-
-if (virXPathUInt("number(./availableInstances)", ctxt,
- &type->available_instances) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("missing number of available instances for "
- "mediated device type '%s'"),
- type->id);
-goto cleanup;
-}
-
-type->name = virXPathString("string(./name)", ctxt);
-
-if (VIR_APPEND_ELEMENT(pci_dev->mdev_types,
- pci_dev->nmdev_types, type) < 0)
-goto cleanup;
-}
-
-pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV;
-ret = 0;
- cleanup:
-VIR_FREE(nodes);
-virMediatedDeviceTypeFree(type);
-ctxt->node = orignode;
-return ret;
-}
-
-
static int
virNodeDevPCICapability
[PATCH 00/10] Make mdev_types generic and support it on CSS devices
Since channel subsystem devices can have the capability to create an mdev device they should expose this by listing the mdev_types capability. Many patches of this series is concerned with refactoring existing PCI code. Boris Fiuczynski (10): conf: node_device: fix mdev_types format and XML parsing code to match schema util: refactor mdev_types method from PCI to mdev util: refactor mdev_types methods return code usage conf: node_devive: refactor GetPCIMdevTypesCaps into GetMdevTypeCapes conf: node_device: refactor capability mdev_types formating conf: node_device: refactor mdev_types XML parsing conf: node_device: refactor CSS formating schema: refactor mdev_types out of PCI nodedev schema conf: node_device: cleanup virNodeDevCapCCWParseXML node_device: detecting mdev_types capability on CSS devices docs/drvnodedev.html.in | 5 +- docs/formatnode.html.in | 39 ++ docs/schemas/nodedev.rng | 52 ++- src/conf/node_device_conf.c | 365 -- src/conf/node_device_conf.h | 11 + src/conf/virnodedeviceobj.c | 7 +- src/libvirt_private.syms | 3 +- src/node_device/node_device_udev.c| 3 + src/util/virmdev.c| 67 src/util/virmdev.h| 5 + src/util/virpci.c | 60 --- src/util/virpci.h | 3 - .../css_0_0_fffe_mdev_types.xml | 17 + tests/nodedevxml2xmltest.c| 1 + 14 files changed, 428 insertions(+), 210 deletions(-) create mode 100644 tests/nodedevschemadata/css_0_0_fffe_mdev_types.xml -- 2.25.1
[PATCH 07/10] conf: node_device: refactor CSS formating
Move XML formating code into a new method.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Marc Hartmayer
Reviewed-by: Bjoern Walk
---
src/conf/node_device_conf.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 6fd5b1b038..ebabf20b67 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -533,6 +533,20 @@ virNodeDeviceCapMdevDefFormat(virBufferPtr buf,
}
}
+
+static void
+virNodeDeviceCapCCWDefFormat(virBufferPtr buf,
+ const virNodeDevCapData *data)
+{
+virBufferAsprintf(buf, "0x%x\n",
+ data->ccw_dev.cssid);
+virBufferAsprintf(buf, "0x%x\n",
+ data->ccw_dev.ssid);
+virBufferAsprintf(buf, "0x%04x\n",
+ data->ccw_dev.devno);
+}
+
+
char *
virNodeDeviceDefFormat(const virNodeDeviceDef *def)
{
@@ -619,12 +633,7 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def)
break;
case VIR_NODE_DEV_CAP_CCW_DEV:
case VIR_NODE_DEV_CAP_CSS_DEV:
-virBufferAsprintf(&buf, "0x%x\n",
- data->ccw_dev.cssid);
-virBufferAsprintf(&buf, "0x%x\n",
- data->ccw_dev.ssid);
-virBufferAsprintf(&buf, "0x%04x\n",
- data->ccw_dev.devno);
+virNodeDeviceCapCCWDefFormat(&buf, data);
break;
case VIR_NODE_DEV_CAP_MDEV_TYPES:
case VIR_NODE_DEV_CAP_FC_HOST:
--
2.25.1
[PATCH] qemu: Don't pass mode when opening domain log file for reading
In qemuDomainLogContextNew() the domain log file is opened.
Twice, the first time for writing, and the second time for
reading (if required by caller). When opening the log file for
reading a mode is provided. This doesn't do much harm, but is
unnecessary. Drop the mode.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 161b369712..d7dbca487a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6251,7 +6251,7 @@ qemuDomainLogContextPtr
qemuDomainLogContextNew(virQEMUDriverPtr driver,
}
if (mode == QEMU_DOMAIN_LOG_CONTEXT_MODE_START) {
-if ((ctxt->readfd = open(ctxt->path, O_RDONLY, S_IRUSR | S_IWUSR))
< 0) {
+if ((ctxt->readfd = open(ctxt->path, O_RDONLY)) < 0) {
virReportSystemError(errno, _("failed to open logfile %s"),
ctxt->path);
goto error;
--
2.26.2
Re: [PATCH] qemu: Don't pass mode when opening domain log file for reading
On Fri, Oct 23, 2020 at 07:42:39PM +0200, Michal Privoznik wrote: > In qemuDomainLogContextNew() the domain log file is opened. > Twice, the first time for writing, and the second time for > reading (if required by caller). When opening the log file for > reading a mode is provided. This doesn't do much harm, but is > unnecessary. Drop the mode. > > Signed-off-by: Michal Privoznik > --- > src/qemu/qemu_domain.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH v5 06/12] nbd: Update qapi to support exporting multiple bitmaps
Since 'nbd-server-add' is deprecated, and 'block-export-add' is new to
5.2, we can still tweak the interface. Allowing 'bitmaps':['str'] is
nicer than 'bitmap':'str'. This wires up the qapi and qemu-nbd
changes to permit passing multiple bitmaps as distinct metadata
contexts that the NBD client may request, but the actual support for
more than one will require a further patch to the server.
Signed-off-by: Eric Blake
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
docs/system/deprecated.rst | 4 +++-
qapi/block-export.json | 18 --
blockdev-nbd.c | 13 +
nbd/server.c | 19 +--
qemu-nbd.c | 10 +-
5 files changed, 46 insertions(+), 18 deletions(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 905628f3a0cb..d6cd027ac740 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -268,7 +268,9 @@ the 'wait' field, which is only applicable to sockets in
server mode
Use the more generic commands ``block-export-add`` and ``block-export-del``
-instead.
+instead. As part of this deprecation, it is now preferred to export a
+list of dirty bitmaps via ``bitmaps``, rather than a single bitmap via
+``bitmap``.
Human Monitor Protocol (HMP) commands
-
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 893d5cde5dfe..c7c749d61097 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -74,10 +74,10 @@
# @description: Free-form description of the export, up to 4096 bytes.
# (Since 5.0)
#
-# @bitmap: Also export the dirty bitmap reachable from @device, so the
-# NBD client can use NBD_OPT_SET_META_CONTEXT with the
-# metadata context name "qemu:dirty-bitmap:NAME" to inspect the
-# bitmap. (since 4.0)
+# @bitmaps: Also export each of the named dirty bitmaps reachable from
+# @device, so the NBD client can use NBD_OPT_SET_META_CONTEXT with
+# the metadata context name "qemu:dirty-bitmap:BITMAP" to inspect
+# each bitmap. (since 5.2)
#
# @allocation-depth: Also export the allocation depth map for @device, so
#the NBD client can use NBD_OPT_SET_META_CONTEXT with
@@ -88,7 +88,7 @@
##
{ 'struct': 'BlockExportOptionsNbd',
'data': { '*name': 'str', '*description': 'str',
-'*bitmap': 'str', '*allocation-depth': 'bool' } }
+'*bitmaps': ['str'], '*allocation-depth': 'bool' } }
##
# @NbdServerAddOptions:
@@ -100,12 +100,18 @@
# @writable: Whether clients should be able to write to the device via the
#NBD connection (default false).
#
+# @bitmap: Also export a single dirty bitmap reachable from @device, so the
+# NBD client can use NBD_OPT_SET_META_CONTEXT with the metadata
+# context name "qemu:dirty-bitmap:BITMAP" to inspect the bitmap
+# (since 4.0). Mutually exclusive with @bitmaps, and newer
+# clients should use that instead.
+#
# Since: 5.0
##
{ 'struct': 'NbdServerAddOptions',
'base': 'BlockExportOptionsNbd',
'data': { 'device': 'str',
-'*writable': 'bool' } }
+'*writable': 'bool', '*bitmap': 'str' } }
##
# @nbd-server-add:
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index cee9134b12eb..cfd46223bf4d 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -192,6 +192,19 @@ void qmp_nbd_server_add(NbdServerAddOptions *arg, Error
**errp)
return;
}
+/*
+ * New code should use the list 'bitmaps'; but until this code is
+ * gone, we must support the older single 'bitmap'. Use only one.
+ */
+if (arg->has_bitmap) {
+if (arg->has_bitmaps) {
+error_setg(errp, "Can't mix 'bitmap' and 'bitmaps'");
+return;
+}
+arg->has_bitmaps = true;
+QAPI_LIST_ADD(arg->bitmaps, g_strdup(arg->bitmap));
+}
+
/*
* block-export-add would default to the node-name, but we may have to use
* the device name as a default here for compatibility.
diff --git a/nbd/server.c b/nbd/server.c
index 30cfe0eee467..884ffa00f1bd 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1495,6 +1495,7 @@ static int nbd_export_create(BlockExport *blk_exp,
BlockExportOptions *exp_args,
uint64_t perm, shared_perm;
bool readonly = !exp_args->writable;
bool shared = !exp_args->writable;
+strList *bitmaps;
int ret;
assert(exp_args->type == BLOCK_EXPORT_TYPE_NBD);
@@ -1556,12 +1557,18 @@ static int nbd_export_create(BlockExport *blk_exp,
BlockExportOptions *exp_args,
}
exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE);
-if (arg->bitmap) {
+/* XXX Allow more than one bitmap */
+if (arg->bitmaps && arg->bitmaps->next) {
+error_setg(errp, "multiple bitmaps per export not supported yet");
+return -EOPNOTSUPP;
+}
+for (bitm
Re: [PATCH 5/6] qemu: Add infrastructure for 'block-export-add' to export NBD
On 10/19/20 3:25 PM, Peter Krempa wrote: > On Mon, Oct 19, 2020 at 13:32:32 -0500, Eric Blake wrote: >> On 10/14/20 5:04 AM, Peter Krempa wrote: >>> Add the monitor code, corresponding generator of properties for NBD and >>> tests validating it against the schema. >>> >>> Signed-off-by: Peter Krempa >>> --- >>> + >>> +if (virJSONValueObjectCreate(&ret, >>> + "s:type", "nbd", >>> + "s:id", exportid, >>> + "s:node-name", nodename, >>> + "b:writable", writable, >>> + "s:name", exportname, >>> + "S:bitmap", bitmap, >>> + NULL) < 0) >> >> The plan is to upgrade to '*bitmaps':['str'], and retiring '*bitmap':'str' >> when nbd-server-add goes away: >> https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg02713.html >> >> so you'll need to tweak this to match. > > Uhh, okay, make sure to get that in before the libvirt release so that > we don't have a broken libvirt version. I'll fix libvirt ASAP. v5 of the qemu patch has now landed, with a goal of making soft freeze next week. And this time, I amended that patch to touch deprecated.rst, so this list gets earlier heads-up notification ;) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 1/1] virt-aa-helper: allow hard links for mounts
On Donnerstag, 22. Oktober 2020 19:07:33 CEST Michal Privoznik wrote: > [Please don't CC random people on patches until asked to, we are all > subscribed to the list] > Got it, I'll refrain from CCing on libvirt in future. Not as erratic as it looks like though: I CCed people who touched this specific AppArmor permission before, plus the virtiofs maintainers. > On 10/22/20 4:58 PM, Christian Schoenebeck wrote: > > Guests should be allowed to create hard links on mounted pathes, since > > many applications rely on this functionality and would error on guest > > with current "rw" AppArmor permission with 9pfs. > > > > Signed-off-by: Christian Schoenebeck > > --- > > > > src/security/virt-aa-helper.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c > > index 12429278fb..5a6f4a5f7d 100644 > > --- a/src/security/virt-aa-helper.c > > +++ b/src/security/virt-aa-helper.c > > @@ -1142,7 +1142,7 @@ get_files(vahControl * ctl) > > > > /* We don't need to add deny rw rules for readonly mounts, > > > >* this can only lead to troubles when mounting / readonly. > >*/ > > > > -if (vah_add_path(&buf, fs->src->path, fs->readonly ? "R" : > > "rw", true) != 0) +if (vah_add_path(&buf, fs->src->path, > > fs->readonly ? "R" : "rwl", true) != 0)> > > goto cleanup; > > > > } > > > > } > > Reviewed-by: Michal Privoznik > > but I will give a day or two for other developers to chime in. > > Michal Yes, please wait couple days to see whether there are reactions. Best regards, Christian Schoenebeck
Re: [PATCH v2 2/7] migration/dirtyrate: Implement qemuMonitorJSONExtractDirtyRateInfo
On 10/19/20 9:58 AM, Hao Wang wrote:
Implement qemuMonitorJSONExtractDirtyRateInfo to deal with the return from
qmp "query-dirty-rate", and store them in virDomainDirtyRateInfo.
Signed-off-by: Hao Wang
Reviewed-by: Chuan Zheng
---
This patch failed to compile in my env:
../src/qemu/qemu_monitor_json.c:9649:1: error:
‘qemuMonitorJSONExtractDirtyRateInfo’ defined but not used
[-Werror=unused-function]
9649 | qemuMonitorJSONExtractDirtyRateInfo(virJSONValuePtr data,
| ^~~
cc1: all warnings being treated as errors
DHB
include/libvirt/libvirt-domain.h | 16 +++
src/qemu/qemu_monitor_json.c | 48
2 files changed, 64 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index c7e22d4af1..9bf4f8a8cf 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -5012,6 +5012,22 @@ int virDomainBackupBegin(virDomainPtr domain,
char *virDomainBackupGetXMLDesc(virDomainPtr domain,
unsigned int flags);
+/**
+ * virDomainDirtyRateStatus:
+ *
+ * Details on the cause of a dirtyrate calculation status.
+ */
+
+typedef enum {
+VIR_DOMAIN_DIRTYRATE_UNSTARTED = 0, /* the dirtyrate calculation has not
been started */
+VIR_DOMAIN_DIRTYRATE_MEASURING = 1, /* the dirtyrate calculation is
measuring */
+VIR_DOMAIN_DIRTYRATE_MEASURED = 2, /* the dirtyrate calculation is
completed */
+
+# ifdef VIR_ENUM_SENTINELS
+VIR_DOMAIN_DIRTYRATE_LAST
+# endif
+} virDomainDirtyRateStatus;
+
/**
* virDomainDirtyRateInfo:
*
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e88e6aebaf..37612a9bba 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9414,3 +9414,51 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
return virJSONValueGetBoolean(virJSONValueObjectGet(reply, "return"),
migratable);
}
+
+
+VIR_ENUM_DECL(qemuDomainDirtyRateStatus);
+VIR_ENUM_IMPL(qemuDomainDirtyRateStatus,
+ VIR_DOMAIN_DIRTYRATE_LAST,
+ "unstarted",
+ "measuring",
+ "measured");
+
+static int
+qemuMonitorJSONExtractDirtyRateInfo(virJSONValuePtr data,
+virDomainDirtyRateInfoPtr info)
+{
+const char *status;
+int statusID;
+
+if (!(status = virJSONValueObjectGetString(data, "status"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'status' data"));
+return -1;
+}
+
+if ((statusID = qemuDomainDirtyRateStatusTypeFromString(status)) < 0) {
+return -1;
+}
+info->status = statusID;
+
+if ((info->status == VIR_DOMAIN_DIRTYRATE_MEASURED) &&
+(virJSONValueObjectGetNumberLong(data, "dirty-rate", &(info->dirtyRate))
< 0)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'dirty-rate'
data"));
+return -1;
+}
+
+if (virJSONValueObjectGetNumberLong(data, "start-time", &(info->startTime))
< 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'start-time'
data"));
+return -1;
+}
+
+if (virJSONValueObjectGetNumberLong(data, "calc-time", &(info->calcTime))
< 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'calc-time'
data"));
+return -1;
+}
+
+return 0;
+}
[libvirt PATCH 2/2] qemu: combine conditionals
Trivial fix to improve readability by combining these into a compound
conditional.
Signed-off-by: Jonathon Jongsma
---
src/qemu/qemu_monitor_json.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 2491cbf9b8..8d3b6c98e3 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4020,9 +4020,9 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitorPtr mon,
if (virJSONValueObjectCreate(&args, "S:opaque", opaque, NULL) < 0)
return -1;
-if (fdset >= 0)
-if (virJSONValueObjectAdd(args, "j:fdset-id", fdset, NULL) < 0)
-return -1;
+if (fdset >= 0 &&
+virJSONValueObjectAdd(args, "j:fdset-id", fdset, NULL) < 0)
+return -1;
if (!(cmd = qemuMonitorJSONMakeCommandInternal("add-fd",
g_steal_pointer(&args
--
2.26.2
[libvirt PATCH 1/2] qemu: fix memory leak reported by coverity
Let g_autoptr clean up on early return.
Signed-off-by: Jonathon Jongsma
---
src/qemu/qemu_monitor_json.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index cba9ec7b19..2491cbf9b8 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4013,7 +4013,7 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitorPtr mon,
const char *opaque,
qemuMonitorAddFdInfoPtr fdinfo)
{
-virJSONValuePtr args = NULL;
+g_autoptr(virJSONValue) args = NULL;
g_autoptr(virJSONValue) reply = NULL;
g_autoptr(virJSONValue) cmd = NULL;
@@ -4024,7 +4024,8 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitorPtr mon,
if (virJSONValueObjectAdd(args, "j:fdset-id", fdset, NULL) < 0)
return -1;
-if (!(cmd = qemuMonitorJSONMakeCommandInternal("add-fd", args)))
+if (!(cmd = qemuMonitorJSONMakeCommandInternal("add-fd",
+ g_steal_pointer(&args
return -1;
if (qemuMonitorJSONCommandWithFd(mon, cmd, fd, &reply) < 0)
--
2.26.2
Re: [libvirt PATCH 1/2] qemu: fix memory leak reported by coverity
On 10/23/20 5:40 PM, Jonathon Jongsma wrote:
Let g_autoptr clean up on early return.
Signed-off-by: Jonathon Jongsma
---
src/qemu/qemu_monitor_json.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index cba9ec7b19..2491cbf9b8 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4013,7 +4013,7 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitorPtr mon,
const char *opaque,
qemuMonitorAddFdInfoPtr fdinfo)
{
-virJSONValuePtr args = NULL;
+g_autoptr(virJSONValue) args = NULL;
g_autoptr(virJSONValue) reply = NULL;
g_autoptr(virJSONValue) cmd = NULL;
@@ -4024,7 +4024,8 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitorPtr mon,
if (virJSONValueObjectAdd(args, "j:fdset-id", fdset, NULL) < 0)
return -1;
-if (!(cmd = qemuMonitorJSONMakeCommandInternal("add-fd", args)))
+if (!(cmd = qemuMonitorJSONMakeCommandInternal("add-fd",
+ g_steal_pointer(&args
return -1;
The examples about curly braces in the coding guidelines say that curly
braces around the body in this case are optional, but the text itself
says that they are required. I'm not sure why this ambiguity exists, but
because combining the two lines of the conditional only increases the
line length to 86 (based on the sentiments expressed during a recent
discussion on that topic on the mailing list), I'm going to do that
instead of adding the curly braces.
Reviewed-by: Laine Stump
and pushed.
if (qemuMonitorJSONCommandWithFd(mon, cmd, fd, &reply) < 0)
Re: [libvirt PATCH 2/2] qemu: combine conditionals
On 10/23/20 5:40 PM, Jonathon Jongsma wrote:
Trivial fix to improve readability by combining these into a compound
conditional.
Signed-off-by: Jonathon Jongsma
---
src/qemu/qemu_monitor_json.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 2491cbf9b8..8d3b6c98e3 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4020,9 +4020,9 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitorPtr mon,
if (virJSONValueObjectCreate(&args, "S:opaque", opaque, NULL) < 0)
return -1;
-if (fdset >= 0)
-if (virJSONValueObjectAdd(args, "j:fdset-id", fdset, NULL) < 0)
-return -1;
+if (fdset >= 0 &&
+virJSONValueObjectAdd(args, "j:fdset-id", fdset, NULL) < 0)
+return -1;
In this case, I've added braces rather than combining the lines, since
each is a separate subexpression, rather than just pieces of a single
simple expression, as was the case in the previous patch.
Reviewed-by: Laine Stump
and pushed.
if (!(cmd = qemuMonitorJSONMakeCommandInternal("add-fd",
g_steal_pointer(&args
Re: [PATCH] qemu: virtiofs: Add pool element to limit thread pool
On Fri, Oct 23, 2020 at 09:10:23AM -0300, Daniel Henrique Barboza wrote:
>
>
> On 10/20/20 2:54 PM, Masayoshi Mizuma wrote:
> > From: Masayoshi Mizuma
> >
> > virtiofsd has --thread-pool-size=NUM option to limit the
> > thread pool size. It will be useful to tune the performance.
> >
> > Add pool element to use the option like as:
> >
> >
> >
> >
> >
> >
> >
> >
> > Signed-off-by: Masayoshi Mizuma
> > ---
>
> Code looks good. We generally split the docs from the parser/qemu logic
> in the commits though. For new options it's also good to have unit tests
> covering them as well.
>
> I suggest you split this patch in at least 2 commits. First commit contains
> the changes in the /docs files, introducing the new 'pool' option. The
> second commit contains the changes in domain_conf and qemu_virtiofs that
> implements it.
>
> For unit tests, you'll want something in the lines of what is done in the
> case of 'vhost-user-fs-fd-memory' test in qemuxml2argvtest.c. You'll need
> a new XML file (e.g. vhost-user-fs-pool.xml) in qemuxml2argvdata that
> implements the 'pool' option, then a .args file that will contains the
> expected QEMU command line generated by that XML. You can see how it is being
> done for 'vhost-user-fs-fd-memory' and go from there. All the extra files
> and changes for this new test goes in another patch.
>
> Last but not the least, if you want to make the maintainers life easier, you
> can also add an extra patch documenting this new feature in NEWS.rst.
Thank you for the helpful advice!
I'll split the patch to the followings:
1. docs/formatdomain.rst and docs/schemas/domaincommon.rng
2. src/conf/domain_conf.[ch] and src/qemu/qemu_virtiofs.c
3. The unit test
4. NEWS.rst
Thanks!
Masa
>
>
>
> Thanks,
>
>
> DHB
>
>
>
>
> > docs/formatdomain.rst | 6 --
> > docs/schemas/domaincommon.rng | 5 +
> > src/conf/domain_conf.c| 12
> > src/conf/domain_conf.h| 1 +
> > src/qemu/qemu_virtiofs.c | 3 +++
> > 5 files changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > index bfa80e4bc2..0f66af5dc3 100644
> > --- a/docs/formatdomain.rst
> > +++ b/docs/formatdomain.rst
> > @@ -3070,7 +3070,7 @@ A directory on the host that can be accessed directly
> > from the guest.
> >
> >
> >
> > -
> > +
> >
> >
> >
> > @@ -3188,7 +3188,9 @@ A directory on the host that can be accessed directly
> > from the guest.
> > the use of filesystem extended attributes. Caching can be tuned via the
> > ``cache`` element, possible ``mode`` values being ``none`` and
> > ``always``.
> > Locking can be controlled via the ``lock`` element - attributes
> > ``posix`` and
> > - ``flock`` both accepting values ``on`` or ``off``. ( :since:`Since
> > 6.2.0` )
> > + ``flock`` both accepting values ``on`` or ``off``. ( :since:`Since
> > 6.2.0` ).
> > + Thread pool size limit can be tuned via the ``pool`` element.
> > + ( :since:`Since 6.9.0` )
> > ``source``
> > The resource on the host that is being accessed in the guest. The
> > ``name``
> > attribute must be used with ``type='template'``, and the ``dir``
> > attribute
> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> > index c25a742581..cd5de2ba80 100644
> > --- a/docs/schemas/domaincommon.rng
> > +++ b/docs/schemas/domaincommon.rng
> > @@ -2839,6 +2839,11 @@
> >
> >
> >
> > +
> > +
> > +
> > +
> > +
> >
> >
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index efa5ac527b..9d06f8c75f 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -11588,6 +11588,7 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
> > g_autofree char *cache =
> > virXPathString("string(./binary/cache/@mode)", ctxt);
> > g_autofree char *posix_lock =
> > virXPathString("string(./binary/lock/@posix)", ctxt);
> > g_autofree char *flock =
> > virXPathString("string(./binary/lock/@flock)", ctxt);
> > +g_autofree char *thread_pool_size =
> > virXPathString("string(./binary/@pool)", ctxt);
> > int val;
> > if (queue_size && virStrToLong_ull(queue_size, NULL, 10,
> > &def->queue_size) < 0) {
> > @@ -11597,6 +11598,14 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr
> > xmlopt,
> > goto error;
> > }
> > +if (thread_pool_size &&
> > +virStrToLong_ull(thread_pool_size, NULL, 10,
> > &def->thread_pool_size) < 0) {
> > +virReportError(VIR_ERR_XML_ERROR,
> > + _("cannot parse thread pool size '%s' for
> > virtiofs"),
> > + thread_pool_size);
> > +
Re: [PATCH 0/6] qemu: Fix cdrom as SCSI hostdev via -blockdev
Greetings Peter, > Sent: Thursday, October 22, 2020 at 8:54 PM > From: "daggs" > To: "Peter Krempa" > Cc: libvir-list@redhat.com > Subject: Re: [PATCH 0/6] qemu: Fix cdrom as SCSI hostdev via -blockdev > > Greetings Peter, > > I've built the os with scsi virtio, now I do see the cdrom, see: > # ls -lia /sys/bus/scsi/devices > total 0 >7017 drwxr-xr-x2 root root 0 Oct 22 2020 . >7015 drwxr-xr-x4 root root 0 Oct 22 2020 .. > 18874 lrwxrwxrwx1 root root 0 Oct 22 2020 0:0:0:0 -> > ../../../devices/pci:00/:00:1e.0/:06:00.0/:07:01.0/virtio2/host0/target0:0:0/0:0:0:0 > 17468 lrwxrwxrwx1 root root 0 Oct 22 2020 host0 -> > ../../../devices/pci:00/:00:1e.0/:06:00.0/:07:01.0/virtio2/host0 > 18051 lrwxrwxrwx1 root root 0 Oct 22 2020 host1 -> > ../../../devices/pci:00/:00:1f.2/ata1/host1 > 18099 lrwxrwxrwx1 root root 0 Oct 22 2020 host2 -> > ../../../devices/pci:00/:00:1f.2/ata2/host2 > 18147 lrwxrwxrwx1 root root 0 Oct 22 2020 host3 -> > ../../../devices/pci:00/:00:1f.2/ata3/host3 > 18195 lrwxrwxrwx1 root root 0 Oct 22 2020 host4 -> > ../../../devices/pci:00/:00:1f.2/ata4/host4 > 18243 lrwxrwxrwx1 root root 0 Oct 22 2020 host5 -> > ../../../devices/pci:00/:00:1f.2/ata5/host5 > 18291 lrwxrwxrwx1 root root 0 Oct 22 2020 host6 -> > ../../../devices/pci:00/:00:1f.2/ata6/host6 > 18835 lrwxrwxrwx1 root root 0 Oct 22 2020 target0:0:0 > -> > ../../../devices/pci:00/:00:1e.0/:06:00.0/:07:01.0/virtio2/host0/target0:0:0 > > I've tested with 6.7.0 and 6.8.0 + your patchset, in both the cdrom is > visible, in 6.7.0, the guest's dmesg is filled with theses errors: > [ 57.510366] sr 0:0:0:0: ioctl_internal_command return code = 802 > [ 57.510400] sr 0:0:0:0: Sense Key : 0xb [current] > [ 57.510403] sr 0:0:0:0: ASC=0x0 ASCQ=0x6 > > every few seconds these lines are added to the log. > with 6.8.0 + your patchset, I see these outputs only few times in the boot > process. here is the dmesg: https://dpaste.com/F32A9B92H > I've inserted a audio cd and expected the os ui to detected it but it didn't. > I assume there is another bug there. > anything I can do to help hunting this bug? looks like I was mistaken, I see the same prints in 6.8.0 + your patchset. this means it is not related, so question is, is this a livbirt misconfig or qemu bug? any insights? Thanks, Dagg.
