Re: [libvirt] [PATCH 07/12] storage: use GRegex virStorageBackendLogicalParseVolExtents
On Wed, Nov 13, 2019 at 16:48:48 +0100, Ján Tomko wrote: > Using GRegex simplifies the code since g_match_info_fetch will > copy the matched substring for us. > > Signed-off-by: Ján Tomko > --- > src/storage/storage_backend_logical.c | 49 +++ > 1 file changed, 13 insertions(+), 36 deletions(-) I'm getting a build failure with this patch: ../../src/storage/storage_backend_logical.c:128:11: error: variable 'p' set but not used [-Werror=unused-but-set-variable] 128 | char *p = NULL; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] Introduce the support of Intel RDT-MBM
Just a reminder that libvirt binds need to be updated after patches introduced. Refer to libvirt python and perl bindings: commit b0a7747 Author: Pavel Hrdina Date: Fri Sep 20 11:14:35 2019 +0200 virDomainMemoryStats: include disk caches Introduced in libvirt 4.6.0 by commit . Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1683516 Signed-off-by: Pavel Hrdina Reviewed-by: Daniel P. Berrangé commit e562e58 Author: Daniel P. Berrangé Date: Wed May 22 14:07:57 2019 +0100 Add new hugetlb memory stats constants Signed-off-by: Daniel P. Berrangé On Wed, Nov 13, 2019 at 6:34 PM Wang Huaqiang wrote: > RDT is the short for Intel Resource Director Technology, consists > of four sub-technologies until now: > > -. CAT for cache allocation > -. CMT for cache usage monitoring > -. MBA for memory bandwidth allocation > -. MBM for memory bandwidth usage monitoring > > The Linux kernel interface is 'resctrl' file system, and we have > already implemented the support of CAT, CMT and MBA, to accomplish > the tasks such as allocating a part of shared CPU last level cache > to particular domain vcpu or a list of vcpus and monitoring the > usage of cache, or the task of allocating a mount of memory > bandwidth to specify domain vcpu(s). > > This series is to introduce the support of MBM. > > Basically the interfaces are: > > ** Identify host capability ** > > Similar to identify the host capability of CMT, it could be gotten > through the result of 'virsh capabilities', if following elements > are found, then MBM is supported: > > > > > > > > > 'mbm_total_bytes' means supporting to report the memory bandwidth > used by the vcpu(s) of specific monitor on all CPU sockets. > > 'mbm_local_bytes' means supporting to report the memory bandwidth > used by vcpu(s) that is passing through local CPU socket. > > ** Create monitor group** > > The monitor group for specific domain vcpus, for example vcpu 0-4, > is defined in domain configuration file, in such kind of way: > > > > > > > > ** Report memory usage ** > > Introduced an option '--memory' against 'virsh domstats' command > to show the memory bandwidth usage in such way: > (also very similar to the format of CMT result.) > > # virsh domstats --memory > > Domain: 'libvirt-vm' > memory.bandwidth.monitor.count=4 > memory.bandwidth.monitor.0.name=vcpus_0-4 > memory.bandwidth.monitor.0.vcpus=0-4 > memory.bandwidth.monitor.0.node.count=2 > memory.bandwidth.monitor.0.node.0.id=0 > memory.bandwidth.monitor.0.node.0.bytes.total=14201651200 > memory.bandwidth.monitor.0.node.0.bytes.local=7369809920 > memory.bandwidth.monitor.0.node.1.id=1 > memory.bandwidth.monitor.0.node.1.bytes.total=188897640448 > memory.bandwidth.monitor.0.node.1.bytes.local=170044047360 > > > Huaqiang (5): > util, resctrl: using 64bit interface instead of 32bit for counters > conf: showing cache/memoryBW monitor features in capabilities > cachetune schema: a looser check for the order of and > element > conf: Parse dommon configure file for memorytune monitors > virsh: show memoryBW info in 'virsh domstats' command > > docs/schemas/domaincommon.rng | 91 +- > include/libvirt/libvirt-domain.h | 1 + > src/conf/capabilities.c| 4 +- > src/conf/domain_conf.c | 44 +++-- > src/libvirt-domain.c | 21 + > src/qemu/qemu_driver.c | 103 - > src/util/virfile.c | 40 > src/util/virfile.h | 2 + > src/util/virresctrl.c | 6 +- > src/util/virresctrl.h | 2 +- > tests/genericxml2xmlindata/cachetune.xml | 1 + > tests/genericxml2xmlindata/memorytune.xml | 5 + > tests/genericxml2xmloutdata/cachetune.xml | 34 +++ > tests/genericxml2xmloutdata/memorytune.xml | 42 + > tests/genericxml2xmltest.c | 4 +- > tools/virsh-domain-monitor.c | 7 ++ > tools/virsh.pod| 23 - > 17 files changed, 367 insertions(+), 63 deletions(-) > create mode 100644 tests/genericxml2xmloutdata/cachetune.xml > create mode 100644 tests/genericxml2xmloutdata/memorytune.xml > > -- > 2.23.0 > > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > > -- Best regards, --- Han Han Quality Engineer Redhat. Email: h...@redhat.com Phone: +861065339333 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] default video device type
On Wed, Nov 13, 2019 at 05:16:51PM -0500, Cole Robinson wrote: On 11/13/19 8:22 AM, Martin Kletzander wrote: On Mon, Oct 07, 2019 at 02:56:11PM +0200, Pavel Mores wrote: Hi, I'm looking into fixing https://bugzilla.redhat.com/show_bug.cgi?id=1668141 (as a short summary, if a graphics device is added to XML that has no video device, libvirt automatically adds a video device which is always of type 'cirrus' - even if the underlying qemu doesn't support cirrus). I'm able to affect the behaviour in question by using qemu capabilities in qemuDomainDeviceVideoDefPostParse(), see proof-of-concept change in [1]. I have a couple of questions though: 1) is this a proper place and approach to fix the bug? I don't think so because the guest ABI could change. Imagine there is an application that just starts a simple VM (with graphics with no model) and out of nowhere (after a long time) they upgrade libvirt and qemu and the VM will look differently. 2) what would be the full specification of expected behaviour? The bug report only states that the video type shouldn't be cirrus but doesn't say what it should be. [2] gives some information about the order of preference of video device types but I was wondering if there are any opinions about this on this list? Reading the BZ it looks to me like virt-manager should allow choosing cirrus and the different default could be chosen in virt-manager. The BZ is not well described, to be honest. My take on the bug was: libvirt shouldn't use a default configuration that we blatantly know will result in unbootable qemu. So, don't use cirrus when qemuCaps tells us that the qemu binary definitely doesn't have cirrus support. Oh, I missed that. I was going through old tagged e-mails with no reply, that's how I got here. If this resulted in unbootable qemu, that's of course something we can and should change. Pavel's v2 series changes the behavior to: * use cirrus if it's available, so current working behavior is maintained * if cirrus is unavailable but vga _is_ available, use that. this fixes the rhel case, but also likely fixes the default for non-x86 archs that didn't ever have cirrus * if cirrus and vga aren't available, explicitly error that the user needs to specify a video model because we didn't find one for them. Which all sounds good to me. Me too! Thanks for the update. Thanks, Cole signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virhostuptime: Add linux stub for musl
On 10/14/19 10:57 AM, Michal Privoznik wrote:
> When we want to know the boot timestamp of the host, we can call
> virHostGetBootTime(). Under the hood, it uses getutxid() which is
> defined by POSIX and properly check for in configure. However,
> musl took a path where it declares the function but instead of
> providing any useful implementation it returns NULL meaning "no
> record found". If that's the case, use our second best option -
> /proc/uptime and a bit of maths.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1760885
>
> Signed-off-by: Michal Privoznik
> ---
> src/util/virhostuptime.c | 64 ++--
> 1 file changed, 61 insertions(+), 3 deletions(-)
>
> diff --git a/src/util/virhostuptime.c b/src/util/virhostuptime.c
> index 62b781acd5..3189074d3d 100644
> --- a/src/util/virhostuptime.c
> +++ b/src/util/virhostuptime.c
> @@ -25,16 +25,68 @@
> #endif
>
> #include "virhostuptime.h"
> +#include "viralloc.h"
> +#include "virfile.h"
> +#include "virlog.h"
> +#include "virstring.h"
> +#include "virtime.h"
> #include "virthread.h"
>
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> +VIR_LOG_INIT("util.virhostuptime");
> +
> static unsigned long long bootTime;
> static int bootTimeErrno;
> static virOnceControl virHostGetBootTimeOnce = VIR_ONCE_CONTROL_INITIALIZER;
>
> -#ifdef HAVE_GETUTXID
> +#if defined(__linux__)
> +# define UPTIME_FILE "/proc/uptime"
> +static int
> +virHostGetBootTimeProcfs(unsigned long long *btime)
> +{
> +unsigned long long now;
> +double up;
> +g_autofree char *buf = NULL;
> +char *tmp;
> +
> +if (virTimeMillisNow() < 0)
> +return -errno;
> +
> +/* 1KiB limit is more than enough. */
> +if (virFileReadAll(UPTIME_FILE, 1024, ) < 0)
> +return -errno;
> +
> +/* buf contains two doubles now:
> + * $uptime $idle_time
> + * We're interested only in the first one */
> +if (!(tmp = strchr(buf, ' '))) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("uptime file has unexpected format '%s'"),
> + buf);
> +return -EINVAL;
> +}
> +
> +*tmp = '\0';
> +
> +if (virStrToDouble(buf, NULL, ) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to parse sched info value '%s'"),
> + buf);
> +return -EINVAL;
> +}
> +
error message here is a copy+paste mistake. syntax-check says you need
to extend POTFILES.in too. With those fixed:
Reviewed-by: Cole Robinson
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] apparmor fixes triggered by multi disk snapshots
On 10/16/19 10:27 AM, Christian Ehrhardt wrote: > Hi, > the bugs [1][2] that made me debug into this actually only need the > last patch (one line), but while coming along I found several > opportunities for minor improvements of the apparmor code in libvirt. > But that way it became a 4 patch series around apparmor. > > [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1746684 > [2]: https://bugs.launchpad.net/libvirt/+bug/1845506 > > Christian Ehrhardt (4): > virt-aa-helper: clarify command line options > apparmor: drop useless call to get_profile_name > apparmor: refactor AppArmorSetSecurityImageLabel > apparmor: let AppArmorSetSecurityImageLabel append rules > > src/security/security_apparmor.c | 52 +++- > src/security/virt-aa-helper.c| 14 + > 2 files changed, 19 insertions(+), 47 deletions(-) > Not runtime tested, but: Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] virt-aa-helper: add rules for shmem devices
On 10/22/19 8:18 AM, Christian Ehrhardt wrote:
> Shared memory devices need qemu to be able to access certain paths
> either for the shared memory directly (mostly ivshmem-plain) or for a
> socket (mostly ivshmem-doorbell).
>
> Add logic to virt-aa-helper to render those apparmor rules based
> on the domain configuration.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1761645
>
> Signed-off-by: Christian Ehrhardt
> ---
> src/security/virt-aa-helper.c | 35 +++
> 1 file changed, 35 insertions(+)
>
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 7d7262ca39..8c261f0010 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -958,6 +958,7 @@ get_files(vahControl * ctl)
> int rc = -1;
> size_t i;
> char *uuid;
> +char *mem_path = NULL;
> char uuidstr[VIR_UUID_STRING_BUFLEN];
> bool needsVfio = false, needsvhost = false, needsgl = false;
>
> @@ -1224,6 +1225,39 @@ get_files(vahControl * ctl)
> }
> }
>
> +for (i = 0; i < ctl->def->nshmems; i++) {
> +if (ctl->def->shmems[i]) {
shmems[i] should never be NULL here except in the case of a serious and
obvious bug elsewhere in libvirt. So this should be removed IMO. Same
goes for all other device list handling in this function, but that's a
separate issue.
Also style comment, IMO in cases where this type of pattern _is_
relevant, it's nicer to do
if (!condition)
continue;
Which saves a lot of indenting
> +virDomainShmemDef *shmem = ctl->def->shmems[i];
> +/* server path can be on any type and overwrites defaults */
> +if (shmem->server.enabled &&
> +shmem->server.chr.data.nix.path) {
> +if (vah_add_file(, shmem->server.chr.data.nix.path,
> +"rw") != 0)
> +goto cleanup;
> +} else {
> +switch (shmem->model) {
> +case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN:
> +/* until exposed, recreate qemuBuildShmemBackendMemProps
> */
> +if (virAsprintf(_path, "/dev/shm/%s", shmem->name) <
> 0)
> +goto cleanup;
virAsprintf is gone in git, you can use g_strdup_printf, which also
means dropping the error checking. See the other conversions patches
With those changes, for the series:
Reviewed-by: Cole Robinson
> +break;
> +case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL:
> +case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM:
> + /* until exposed, recreate
> qemuDomainPrepareShmemChardev */
> +if (virAsprintf(_path,
> "/var/lib/libvirt/shmem-%s-sock",
> +shmem->name) < 0)
> +goto cleanup;
> +break;
> +}
> +if (mem_path != NULL) {
> +if (vah_add_file(, mem_path, "rw") != 0)
> +goto cleanup;
> +}
> +}
> +}
> +}
> +
> +
> if (ctl->def->tpm) {
> char *shortName = NULL;
> const char *tpmpath = NULL;
> @@ -1324,6 +1358,7 @@ get_files(vahControl * ctl)
> ctl->files = virBufferContentAndReset();
>
> cleanup:
> +VIR_FREE(mem_path);
> VIR_FREE(uuid);
> return rc;
> }
>
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/5] conf: report errors when parsing video resolution
On Wed, 2019-11-13 at 13:54 -0500, Cole Robinson wrote:
> I pushed the first three patches. Comments below
>
> On 10/23/19 1:46 PM, Jonathon Jongsma wrote:
> > The current code doesn't properly handle errors when parsing a
> > video
> > device's resolution.
> >
> > This patch changes the parse function signature to return an error
> > when we're missing an 'x' or 'y' parameter or when the 'x' or 'y'
> > parameters are not positive integers. No error is returned when no
> > 'resolution' element is found.
> >
> > Previously we were returning a NULL structure for the case where
> > 'x' or
> > 'y' were missing, but were returning an under-specified structure
> > for
> > the other error cases.
> >
> > Signed-off-by: Jonathon Jongsma
> > ---
> > src/conf/domain_conf.c | 44
> > --
> > 1 file changed, 30 insertions(+), 14 deletions(-)
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 269a6ec2c3..b3f32d0b63 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -15338,45 +15338,57 @@ virDomainVideoAccelDefParseXML(xmlNodePtr
> > node)
> > return g_steal_pointer();
> > }
> >
> > -static virDomainVideoResolutionDefPtr
> > -virDomainVideoResolutionDefParseXML(xmlNodePtr node)
> > +static int
> > +virDomainVideoResolutionDefParseXML(xmlNodePtr node,
> > +virDomainVideoResolutionDefPtr
> > *res)
> > {
> > xmlNodePtr cur;
> > g_autofree virDomainVideoResolutionDefPtr def = NULL;
> > g_autofree char *x = NULL;
> > g_autofree char *y = NULL;
> >
> > +*res = NULL;
> > cur = node->children;
> > while (cur != NULL) {
> > -if (cur->type == XML_ELEMENT_NODE) {
> > -if (!x && !y &&
> > -virXMLNodeNameEqual(cur, "resolution")) {
> > -x = virXMLPropString(cur, "x");
> > -y = virXMLPropString(cur, "y");
> > -}
> > +if ((cur->type == XML_ELEMENT_NODE) &&
> > +virXMLNodeNameEqual(cur, "resolution")) {
> > +x = virXMLPropString(cur, "x");
> > +y = virXMLPropString(cur, "y");
> > +break;
> > }
> > cur = cur->next;
> > }
> >
>
> This loop can be removed if you move the virXMLNodeNameEqual to the
> parent function, like how it is handled for parent call of
> virDomainVirtioOptionsParseXML
>
> > +/* resolution not specified */
> > +if (cur == NULL)
> > +return 0;
> > +
> > +/* resolution specified, but x or y is missing. report error
> > */
> > if (!x || !y)
> > -return NULL;
> > +return -1;
> >
> > def = g_new0(virDomainVideoResolutionDef, 1);
> >
> > if (virStrToLong_uip(x, NULL, 10, >x) < 0) {
> > virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > _("cannot parse video x-resolution '%s'"),
> > x);
> > -goto cleanup;
> > +return -1;
> > }
> >
> > if (virStrToLong_uip(y, NULL, 10, >y) < 0) {
> > virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > _("cannot parse video y-resolution '%s'"),
> > y);
> > -goto cleanup;
> > +return -1;
> > }
> >
> > - cleanup:
> > -return g_steal_pointer();
> > +if (def->x == 0 || def->y == 0) {
> > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > + _("resolution values must be greater than
> > 0"));
> > +return -1;
> > +}
> > +
> > +*res = g_steal_pointer();
> > +return 0;
> > }
> >
>
> This patch is doing two things: converting to the more common error
> handling pattern, but also adding this error check. Separating them
> will
> help review.
>
> It's borderline pedantic but this type of error should actually be in
> the Validate callback machinery instead. Some drivers will fill in a
> virDomainDef manually in code (like virtualbox). If they accidentally
> set an x or y value of 0, Validate won't catch it as an error, but
> the
> XML parser will catch it later. Generally the XML parser should only
> throw errors about
It seems that this sentence is unfinished?
>
> > static virDomainVideoDriverDefPtr
> > @@ -15458,7 +15470,11 @@
> > virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
> > }
> >
> > def->accel = virDomainVideoAccelDefParseXML(cur);
> > -def->res =
> > virDomainVideoResolutionDefParseXML(cur);
> > +if (virDomainVideoResolutionDefParseXML(cur,
> > >res) < 0) {
> > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > + "%s", _("Cannot parse video
> > resolution"));
> > +goto error;
> > +}
> > }
>
> Calling virReporError here will overwrite the error raised by
> virDomainVideoResolutionDefParseXML. The error reporting machinery
> doesn't have any smarts to
Re: [libvirt] [RFC] default video device type
On 11/13/19 8:22 AM, Martin Kletzander wrote: > On Mon, Oct 07, 2019 at 02:56:11PM +0200, Pavel Mores wrote: >> >> Hi, >> >> I'm looking into fixing >> >> https://bugzilla.redhat.com/show_bug.cgi?id=1668141 >> >> (as a short summary, if a graphics device is added to XML that has no >> video >> device, libvirt automatically adds a video device which is always of type >> 'cirrus' - even if the underlying qemu doesn't support cirrus). >> >> I'm able to affect the behaviour in question by using qemu >> capabilities in >> qemuDomainDeviceVideoDefPostParse(), see proof-of-concept change in >> [1]. I >> have a couple of questions though: >> >> 1) is this a proper place and approach to fix the bug? > > I don't think so because the guest ABI could change. Imagine there is an > application that just starts a simple VM (with graphics with no model) > and out > of nowhere (after a long time) they upgrade libvirt and qemu and the VM > will > look differently. > >> 2) what would be the full specification of expected behaviour? The >> bug report >> only states that the video type shouldn't be cirrus but doesn't say >> what it >> should be. [2] gives some information about the order of preference >> of video >> device types but I was wondering if there are any opinions about >> this on this >> list? >> > > Reading the BZ it looks to me like virt-manager should allow choosing > cirrus and > the different default could be chosen in virt-manager. The BZ is not well > described, to be honest. My take on the bug was: libvirt shouldn't use a default configuration that we blatantly know will result in unbootable qemu. So, don't use cirrus when qemuCaps tells us that the qemu binary definitely doesn't have cirrus support. Pavel's v2 series changes the behavior to: * use cirrus if it's available, so current working behavior is maintained * if cirrus is unavailable but vga _is_ available, use that. this fixes the rhel case, but also likely fixes the default for non-x86 archs that didn't ever have cirrus * if cirrus and vga aren't available, explicitly error that the user needs to specify a video model because we didn't find one for them. Which all sounds good to me. Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] qemu: fix type of default video device
Sorry for the review delay. If you cc me on v3 I'll review it ASAP
This patch is mixing two things: refactoring, and behavior change. This
makes review more difficult. Please put the refactoring first which will
maintain the old behavior, then add functional changes as additional
patch(es).
On its own, this patch breaks the test suite. Every patch should be self
contained, keeping the test suite working.
On 10/18/19 6:27 AM, Pavel Mores wrote:
> If a graphics device is added to XML that has no video device, libvirt
> automatically added a video device which was always of type 'cirrus' on
> x86_64, even if the underlying qemu didn't support cirrus.
>
> This patch refines a bit the decision about the type of the video device.
> Based on QEMU capabilities, cirrus is still preferred but only added if
> QEMU supports it, otherwise VGA is used if supported by QEMU. There is now
> no fallback as libvirt only aspires to generate a basic working config and
> leaves anything more specific up to higher-level management tools.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1668141
> Signed-off-by: Pavel Mores
> ---
> src/qemu/qemu_domain.c | 39 +++
> 1 file changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index b257db44b0..39b2d2da82 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7360,6 +7360,28 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
> }
>
>
> +static int
> +qemuDomainDefaultVideoDevice(const virDomainDef *def,
> + virQEMUCapsPtr qemuCaps)
> +{
> +if (ARCH_IS_PPC64(def->os.arch)) {
> +return VIR_DOMAIN_VIDEO_TYPE_VGA;
> +} else if (qemuDomainIsARMVirt(def) ||
> + qemuDomainIsRISCVVirt(def) ||
> + ARCH_IS_S390(def->os.arch)) {
Indentation is off here.
> +return VIR_DOMAIN_VIDEO_TYPE_VIRTIO;
> +} else {
> +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) {
> +return VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
> +} else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) {
> +return VIR_DOMAIN_VIDEO_TYPE_VGA;
> +} else {
> +return VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
> +}
> +}
> +}
> +
You can remove any 'else' usage after 'return;' here. Every condition
will then be at the same indent level. Then you can drop all the bracket
usage too in accordance with the style guidelines. All this can be done
in the same patch creating the new function IMO.
Second patch can add qemuCaps up through the callstack and make the
behavior change.
The can probably resolve the test breakage by moving current patch #2 to
be first in the series. Then the behavior change patch will demonstrate
the new behavior in changed test suite output, which makes it easier to
review exactly the effect of the changes.
Thanks,
Cole
> +
> /*
> * Clear auto generated unix socket paths:
> *
> @@ -7821,18 +7843,11 @@ qemuDomainDeviceNetDefPostParse(virDomainNetDefPtr
> net,
>
> static int
> qemuDomainDeviceVideoDefPostParse(virDomainVideoDefPtr video,
> - const virDomainDef *def)
> + const virDomainDef *def,
> + virQEMUCapsPtr qemuCaps)
> {
> -if (video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) {
> -if (ARCH_IS_PPC64(def->os.arch))
> -video->type = VIR_DOMAIN_VIDEO_TYPE_VGA;
> -else if (qemuDomainIsARMVirt(def) ||
> - qemuDomainIsRISCVVirt(def) ||
> - ARCH_IS_S390(def->os.arch))
> -video->type = VIR_DOMAIN_VIDEO_TYPE_VIRTIO;
> -else
> -video->type = VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
> -}
> +if (video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT)
> +video->type = qemuDomainDefaultVideoDevice(def, qemuCaps);
>
> if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL &&
> !video->vgamem) {
> @@ -7926,7 +7941,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> break;
>
> case VIR_DOMAIN_DEVICE_VIDEO:
> -ret = qemuDomainDeviceVideoDefPostParse(dev->data.video, def);
> +ret = qemuDomainDeviceVideoDefPostParse(dev->data.video, def,
> qemuCaps);
> break;
>
> case VIR_DOMAIN_DEVICE_PANIC:
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] Add API to change qemu agent response timeout
Some layered products such as oVirt have requested a way to avoid being
blocked by guest agent commands when querying a loaded vm. For example,
many guest agent commands are polled periodically to monitor changes,
and rather than blocking the calling process, they'd prefer to simply
time out when an agent query is taking too long.
This patch adds a way for the user to specify a custom agent timeout
that is applied to all agent commands.
One special case to note here is the 'guest-sync' command. 'guest-sync'
is issued internally prior to calling any other command. (For example,
when libvirt wants to call 'guest-get-fsinfo', we first call
'guest-sync' and then call 'guest-get-fsinfo').
Previously, the 'guest-sync' command used a 5-second timeout
(VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT), whereas the actual command that
followed always blocked indefinitely
(VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK). As part of this patch, if a
custom timeout is specified that is shorter than
5 seconds, this new timeout is also used for 'guest-sync'. If there is
no custom timeout or if the custom timeout is longer than 5 seconds, we
will continue to use the 5-second timeout.
Signed-off-by: Jonathon Jongsma
---
Changes in v3:
- Changed enum names per Daniel's suggestion
- incorporated Michal's review and fixup patches
- As discussed on the mailing list, this patch does not acquire any jobs, but
simply uses mutex locking for data integrity
- libvirt release versions updated
- format and parse private agentTimeout data in status xml
include/libvirt/libvirt-domain.h | 10 +++
include/libvirt/libvirt-qemu.h| 8 +--
src/driver-hypervisor.h | 6 ++
src/libvirt-domain.c | 49 +
src/libvirt_public.syms | 5 ++
src/qemu/qemu_agent.c | 70 ++-
src/qemu/qemu_agent.h | 3 +
src/qemu/qemu_domain.c| 6 ++
src/qemu/qemu_domain.h| 1 +
src/qemu/qemu_driver.c| 47 +
src/remote/remote_driver.c| 1 +
src/remote/remote_protocol.x | 18 -
src/remote_protocol-structs | 9 +++
.../blockjob-blockdev-in.xml | 1 +
.../blockjob-mirror-in.xml| 1 +
.../disk-secinfo-upgrade-out.xml | 1 +
.../migration-in-params-in.xml| 1 +
.../migration-out-nbd-out.xml | 1 +
.../migration-out-nbd-tls-out.xml | 1 +
.../migration-out-params-in.xml | 1 +
tests/qemustatusxml2xmldata/modern-in.xml | 1 +
.../qemustatusxml2xmldata/vcpus-multi-in.xml | 1 +
tools/virsh-domain.c | 52 ++
23 files changed, 257 insertions(+), 37 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 22277b0a84..a2f007568c 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -4916,4 +4916,14 @@ int virDomainGetGuestInfo(virDomainPtr domain,
int *nparams,
unsigned int flags);
+typedef enum {
+VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK = -2,
+VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_DEFAULT = -1,
+VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_NOWAIT = 0,
+} virDomainAgentResponseTimeoutValues;
+
+int virDomainAgentSetResponseTimeout(virDomainPtr domain,
+ int timeout,
+ unsigned int flags);
+
#endif /* LIBVIRT_DOMAIN_H */
diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
index 891617443f..0cc2872821 100644
--- a/include/libvirt/libvirt-qemu.h
+++ b/include/libvirt/libvirt-qemu.h
@@ -43,10 +43,10 @@ virDomainPtr virDomainQemuAttach(virConnectPtr domain,
unsigned int flags);
typedef enum {
-VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN = -2,
-VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2,
-VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1,
-VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0,
+VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN =
VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK,
+VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK =
VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK,
+VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT =
VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_DEFAULT,
+VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT =
VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_NOWAIT,
VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN = 60,
} virDomainQemuAgentCommandTimeoutValues;
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 015b2cd01c..4afd8f6ec5 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1372,6 +1372,11 @@ typedef int
int *nparams,
unsigned int flags);
+typedef int
+(*virDrvDomainAgentSetResponseTimeout)(virDomainPtr
Re: [libvirt] [PATCH v3 0/3] unplug timeout changes for PPC64
On 10/18/19 2:36 PM, Daniel Henrique Barboza wrote: > changes in v3: > - redesigned patch 1 based on Cole Robinson feedback > > v2: https://www.redhat.com/archives/libvir-list/2019-September/msg00447.html > v1: https://www.redhat.com/archives/libvir-list/2019-August/msg00698.html > > > Daniel Henrique Barboza (3): > qemu_hotplug.c: adding qemuDomainGetUnplugTimeout > qemu: Remove qemu_hotplugpriv.h and qemuDomainRemoveDeviceWaitTime > qemu_hotplug.c: user-friendlier setvcpus timeout error message > > src/qemu/Makefile.inc.am | 1 - > src/qemu/qemu_hotplug.c | 28 ++- > src/qemu/qemu_hotplug.h | 2 ++ > tests/Makefile.am | 13 - > .../qemuhotplugmock.c | 27 +- > tests/qemuhotplugtest.c | 7 ++--- > 6 files changed, 51 insertions(+), 27 deletions(-) > rename src/qemu/qemu_hotplugpriv.h => tests/qemuhotplugmock.c (61%) > Reviewed and pushed. Sorry for the delay. One comment though: patch #1 was a mix of refactoring, and the PPC timeout change. In the future please separate changes like that into two patches Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] default video device type
On Mon, Oct 07, 2019 at 02:56:11PM +0200, Pavel Mores wrote: Hi, I'm looking into fixing https://bugzilla.redhat.com/show_bug.cgi?id=1668141 (as a short summary, if a graphics device is added to XML that has no video device, libvirt automatically adds a video device which is always of type 'cirrus' - even if the underlying qemu doesn't support cirrus). I'm able to affect the behaviour in question by using qemu capabilities in qemuDomainDeviceVideoDefPostParse(), see proof-of-concept change in [1]. I have a couple of questions though: 1) is this a proper place and approach to fix the bug? I don't think so because the guest ABI could change. Imagine there is an application that just starts a simple VM (with graphics with no model) and out of nowhere (after a long time) they upgrade libvirt and qemu and the VM will look differently. 2) what would be the full specification of expected behaviour? The bug report only states that the video type shouldn't be cirrus but doesn't say what it should be. [2] gives some information about the order of preference of video device types but I was wondering if there are any opinions about this on this list? Reading the BZ it looks to me like virt-manager should allow choosing cirrus and the different default could be chosen in virt-manager. The BZ is not well described, to be honest. Martin signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Warn verbosely if using old loader:nvram pairs
On 11/13/19 9:20 AM, Michal Privoznik wrote: > On 11/12/19 11:17 PM, Jim Fehlig wrote: >> On 11/11/19 9:42 AM, Michal Privoznik wrote: >>> There are two ways for specifying loader:nvram pairs: >>> >>> 1) --with-loader-nvram configure option >>> 2) nvram variable in qemu.conf >>> >>> Since we have FW descriptors, using this old style is >>> discouraged, but not as strong as one would expect. Produce more >>> warnings: >> >> Oh, I didn't know the old style was discouraged when using FW descriptors. >> Thanks for mentioning it! >> >>> >>> 1) produce a warning if somebody tries the configure option >>> 2) produce a warning if somebody sets nvram variable and at >>> least on FW descriptor was found >>> >>> The reason for producing warning in case 1) is that package >>> maintainers, who set the configure option in the first place >>> should start moving towards FW descriptors and abandon the >>> configure option. After all, the warning is printed into config >>> output only in this case. >> >> Should the configure option be removed from the upstream spec file? > > Definitely, Fedora ships FW descriptors and so does RHEL. What's the state in > OpenSUSE? But I bet you have your own spec file anyway, don't you? Just like > we > have for Fedora and RHEL. Will post a patch tomorrow (unless you beat me to > it). I was attempting to beat you to it, but removing $LOADERS from the spec file means removing use of --with-loader-nvram configure option. Does that mean we should remove associated code from configure.ac, nuke m4/virt-loader-nvram.m4, remove DEFAULT_LOADER_NVRAM from qemu and libxl drivers, ...? It seems a bit early for that since you are just now adding the deprecation warning to virt-loader-nvram.m4 :-). Should we wait until completely removing support for the build-time FW list before nuking from the spec file? Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/5] conf: report errors when parsing video acceleration
On 10/23/19 1:46 PM, Jonathon Jongsma wrote:
> In order to differentiate between a configuration that does not specify
> any video acceleration and one that is incorrectly specified, change the
> signature of virDomainVideoAccelDefParseXML() to return an error
> response.
>
> If any of the values are invalid, report an error rather than returning
> a partially-specified accel object. If no 'acceleration' node is found,
> do not report an error since it is optional.
>
> Signed-off-by: Jonathon Jongsma
> ---
> src/conf/domain_conf.c | 38 +++---
> 1 file changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b3f32d0b63..1c3fc5c4ce 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15285,8 +15285,8 @@ virDomainVideoDefaultType(const virDomainDef *def)
> }
> }
>
> -static virDomainVideoAccelDefPtr
> -virDomainVideoAccelDefParseXML(xmlNodePtr node)
> +static int
> +virDomainVideoAccelDefParseXML(xmlNodePtr node, virDomainVideoAccelDefPtr
> *accel)
> {
> xmlNodePtr cur;
> g_autofree virDomainVideoAccelDefPtr def = NULL;
> @@ -15295,21 +15295,25 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
> g_autofree char *accel3d = NULL;
> g_autofree char *rendernode = NULL;
>
> +*accel = NULL;
> cur = node->children;
> while (cur != NULL) {
> -if (cur->type == XML_ELEMENT_NODE) {
> -if (!accel3d && !accel2d &&
> -virXMLNodeNameEqual(cur, "acceleration")) {
> -accel3d = virXMLPropString(cur, "accel3d");
> -accel2d = virXMLPropString(cur, "accel2d");
> -rendernode = virXMLPropString(cur, "rendernode");
> -}
> +if ((cur->type == XML_ELEMENT_NODE) &&
> +virXMLNodeNameEqual(cur, "acceleration")) {
> +accel3d = virXMLPropString(cur, "accel3d");
> +accel2d = virXMLPropString(cur, "accel2d");
> +rendernode = virXMLPropString(cur, "rendernode");
> +break;
> }
> cur = cur->next;
> }
>
THe virXMLNodeNameEqual check should be moved to the parent, same as
mentioned in patch 4
> +/* no acceleration node was specified */
> +if (cur == NULL)
> +return 0;
> +
> if (!accel3d && !accel2d && !rendernode)
> -return NULL;
> +return -1;
>
> def = g_new0(virDomainVideoAccelDef, 1);
>
> @@ -15317,7 +15321,7 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
> if ((val = virTristateBoolTypeFromString(accel3d)) <= 0) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("unknown accel3d value '%s'"), accel3d);
> -goto cleanup;
> +return -1;
> }
> def->accel3d = val;
> }
> @@ -15326,7 +15330,7 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
> if ((val = virTristateBoolTypeFromString(accel2d)) <= 0) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("unknown accel2d value '%s'"), accel2d);
> -goto cleanup;
> +return -1;
> }
> def->accel2d = val;
> }
> @@ -15334,8 +15338,8 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
> if (rendernode)
> def->rendernode = virFileSanitizePath(rendernode);
>
> - cleanup:
> -return g_steal_pointer();
> +*accel = g_steal_pointer();
> +return 0;
> }
>
> static int
> @@ -15469,7 +15473,11 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr
> xmlopt,
> VIR_FREE(primary);
> }
>
> -def->accel = virDomainVideoAccelDefParseXML(cur);
> +if (virDomainVideoAccelDefParseXML(cur, >accel) < 0) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + "%s", _("Cannot parse video
> acceleration"));
> +goto error;
> +}
> if (virDomainVideoResolutionDefParseXML(cur, >res) < 0)
> {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> "%s", _("Cannot parse video resolution"));
>
Same error overwriting issue here as in patch 4
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/5] conf: report errors when parsing video resolution
I pushed the first three patches. Comments below
On 10/23/19 1:46 PM, Jonathon Jongsma wrote:
> The current code doesn't properly handle errors when parsing a video
> device's resolution.
>
> This patch changes the parse function signature to return an error
> when we're missing an 'x' or 'y' parameter or when the 'x' or 'y'
> parameters are not positive integers. No error is returned when no
> 'resolution' element is found.
>
> Previously we were returning a NULL structure for the case where 'x' or
> 'y' were missing, but were returning an under-specified structure for
> the other error cases.
>
> Signed-off-by: Jonathon Jongsma
> ---
> src/conf/domain_conf.c | 44 --
> 1 file changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 269a6ec2c3..b3f32d0b63 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15338,45 +15338,57 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
> return g_steal_pointer();
> }
>
> -static virDomainVideoResolutionDefPtr
> -virDomainVideoResolutionDefParseXML(xmlNodePtr node)
> +static int
> +virDomainVideoResolutionDefParseXML(xmlNodePtr node,
> +virDomainVideoResolutionDefPtr *res)
> {
> xmlNodePtr cur;
> g_autofree virDomainVideoResolutionDefPtr def = NULL;
> g_autofree char *x = NULL;
> g_autofree char *y = NULL;
>
> +*res = NULL;
> cur = node->children;
> while (cur != NULL) {
> -if (cur->type == XML_ELEMENT_NODE) {
> -if (!x && !y &&
> -virXMLNodeNameEqual(cur, "resolution")) {
> -x = virXMLPropString(cur, "x");
> -y = virXMLPropString(cur, "y");
> -}
> +if ((cur->type == XML_ELEMENT_NODE) &&
> +virXMLNodeNameEqual(cur, "resolution")) {
> +x = virXMLPropString(cur, "x");
> +y = virXMLPropString(cur, "y");
> +break;
> }
> cur = cur->next;
> }
>
This loop can be removed if you move the virXMLNodeNameEqual to the
parent function, like how it is handled for parent call of
virDomainVirtioOptionsParseXML
> +/* resolution not specified */
> +if (cur == NULL)
> +return 0;
> +
> +/* resolution specified, but x or y is missing. report error */
> if (!x || !y)
> -return NULL;
> +return -1;
>
> def = g_new0(virDomainVideoResolutionDef, 1);
>
> if (virStrToLong_uip(x, NULL, 10, >x) < 0) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("cannot parse video x-resolution '%s'"), x);
> -goto cleanup;
> +return -1;
> }
>
> if (virStrToLong_uip(y, NULL, 10, >y) < 0) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("cannot parse video y-resolution '%s'"), y);
> -goto cleanup;
> +return -1;
> }
>
> - cleanup:
> -return g_steal_pointer();
> +if (def->x == 0 || def->y == 0) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("resolution values must be greater than 0"));
> +return -1;
> +}
> +
> +*res = g_steal_pointer();
> +return 0;
> }
>
This patch is doing two things: converting to the more common error
handling pattern, but also adding this error check. Separating them will
help review.
It's borderline pedantic but this type of error should actually be in
the Validate callback machinery instead. Some drivers will fill in a
virDomainDef manually in code (like virtualbox). If they accidentally
set an x or y value of 0, Validate won't catch it as an error, but the
XML parser will catch it later. Generally the XML parser should only
throw errors about
> static virDomainVideoDriverDefPtr
> @@ -15458,7 +15470,11 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr
> xmlopt,
> }
>
> def->accel = virDomainVideoAccelDefParseXML(cur);
> -def->res = virDomainVideoResolutionDefParseXML(cur);
> +if (virDomainVideoResolutionDefParseXML(cur, >res) < 0)
> {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + "%s", _("Cannot parse video resolution"));
> +goto error;
> +}
> }
Calling virReporError here will overwrite the error raised by
virDomainVideoResolutionDefParseXML. The error reporting machinery
doesn't have any smarts to merge errors or anything like that, it needs
to be done manually. In this case you can just drop the virReportError
entirely
I'll be quicker about reviewing v3.
Thanks,
Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 6/6] remote/rpc: Use virNetServerGetProgram() to determine the program
On Wed, Nov 13, 2019 at 09:52 AM +0100, Pavel Hrdina wrote:
> On Fri, Nov 01, 2019 at 06:35:48PM +0100, Marc Hartmayer wrote:
>> Use virNetServerGetProgram() to determine the virNetServerProgram
>> instead of using hard coded global variables. This allows us to remove
>> the global variables @remoteProgram and @qemuProgram as they're now no
>> longer necessary.
>>
>> Signed-off-by: Marc Hartmayer
[…snip…]
>> virNetMessageErrorPtr rerr)
>> {
>> int rv = -1;
>> @@ -4180,6 +4180,12 @@
>> remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server
>> G_GNUC_UNUSED,
>> struct daemonClientPrivate *priv =
>> virNetServerClientGetPrivateData(client);
>> virConnectPtr conn = remoteGetHypervisorConn(client);
>> +virNetServerProgramPtr program;
>> +
>> +if (!(program = virNetServerGetProgram(server, msg))) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no matching program
>> found"));
>> +goto cleanup;
>> +}
>
> This doesn't look right. If the function fails we will jump to cleanup
> where we will try to unlock >lock. This has to happen after we
> acquire that lock.
>
> Pavel
Yep, will fix that as well. Shall I directly return in the error case or
jump to another label (e.g. 'cleanup_unlock')?
Or do see any reason why we should hold the priv->lock during the
virNetServerGetProgram call?
--
Kind regards / Beste Grüße
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/6] virConnectRegisterCloseCallback: Cleanup 'opaque' if there is no connectRegisterCloseCallback
On Fri, Nov 08, 2019 at 04:52 PM +0100, Pavel Hrdina wrote:
> On Fri, Nov 01, 2019 at 06:35:44PM +0100, Marc Hartmayer wrote:
>> The commit 'close callback: move it to driver' (88f09b75eb99) moved
>> the responsibility for the close callback to the driver. But if the
>> driver doesn't support the connectRegisterCloseCallback API this
>> function does nothing, even no unsupported error report. This behavior
>> may lead to problems, for example memory leaks, as the caller cannot
>> differentiate whether the close callback was 'really' registered or
>> not.
>>
>> Therefore call directly @freecb if the connectRegisterCloseCallback
>> API is not supported by the driver used by the connection.
>>
>> Signed-off-by: Marc Hartmayer
>> ---
>> src/libvirt-host.c | 12
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/libvirt-host.c b/src/libvirt-host.c
>> index 221a1b7a4353..9c3a19f33b12 100644
>> --- a/src/libvirt-host.c
>> +++ b/src/libvirt-host.c
>> @@ -1398,7 +1398,7 @@ virConnectIsAlive(virConnectPtr conn)
>> * @conn: pointer to connection object
>> * @cb: callback to invoke upon close
>> * @opaque: user data to pass to @cb
>> - * @freecb: callback to free @opaque
>> + * @freecb: callback to free @opaque when not used anymore
>> *
>> * Registers a callback to be invoked when the connection
>> * is closed. This callback is invoked when there is any
>> @@ -1428,9 +1428,13 @@ virConnectRegisterCloseCallback(virConnectPtr conn,
>> virCheckConnectReturn(conn, -1);
>> virCheckNonNullArgGoto(cb, error);
>>
>> -if (conn->driver->connectRegisterCloseCallback &&
>> -conn->driver->connectRegisterCloseCallback(conn, cb, opaque,
>> freecb) < 0)
>> -goto error;
>> +if (conn->driver->connectRegisterCloseCallback) {
>> +if (conn->driver->connectRegisterCloseCallback(conn, cb, opaque,
>> freecb) < 0)
>> +goto error;
>> +} else {
>> +if (freecb)
>> +freecb(opaque);
>> +}
>
> Looks good but I think we should improve the documentation of this API
> to explicitly state that the @freecb is called only on success and if
> the API fails the caller is responsible to free the opaque data.
Will change it in v4.
>
> Pavel
--
Kind regards / Beste Grüße
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Warn verbosely if using old loader:nvram pairs
On 11/13/19 9:20 AM, Michal Privoznik wrote: > On 11/12/19 11:17 PM, Jim Fehlig wrote: >> On 11/11/19 9:42 AM, Michal Privoznik wrote: >>> There are two ways for specifying loader:nvram pairs: >>> >>> 1) --with-loader-nvram configure option >>> 2) nvram variable in qemu.conf >>> >>> Since we have FW descriptors, using this old style is >>> discouraged, but not as strong as one would expect. Produce more >>> warnings: >> >> Oh, I didn't know the old style was discouraged when using FW descriptors. >> Thanks for mentioning it! >> >>> >>> 1) produce a warning if somebody tries the configure option >>> 2) produce a warning if somebody sets nvram variable and at >>> least on FW descriptor was found >>> >>> The reason for producing warning in case 1) is that package >>> maintainers, who set the configure option in the first place >>> should start moving towards FW descriptors and abandon the >>> configure option. After all, the warning is printed into config >>> output only in this case. >> >> Should the configure option be removed from the upstream spec file? > > Definitely, Fedora ships FW descriptors and so does RHEL. What's the state in > OpenSUSE? Yes, openSUSE supports FW descriptors, as does the latest version of SLES under development (SLES15 SP2). > But I bet you have your own spec file anyway, don't you? Just like we > have for Fedora and RHEL. Will post a patch tomorrow (unless you beat me to > it). Yes, I have my own spec file, heavily influenced by the upstream one :-). Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/4] syntax-check: forbid usage of snprintf
On Wed, Nov 13, 2019 at 10:44:18AM -0600, Eric Blake wrote: > On 11/13/19 10:33 AM, Pavel Hrdina wrote: > > Signed-off-by: Pavel Hrdina > > --- > > > > new in v2 > > > > build-aux/syntax-check.mk | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk > > index ff1198ca73..78458dce53 100644 > > --- a/build-aux/syntax-check.mk > > +++ b/build-aux/syntax-check.mk > > @@ -469,6 +469,11 @@ sc_prohibit_sprintf: > > halt='use g_snprintf, not sprintf' \ > > $(_sc_search_regexp) > > +sc_prohibit_snprintf: > > + @prohibit='\' \ > > + halt='use g_snprintf, not snprintf' \ > > + $(_sc_search_regexp) > > + > > Ah, you added a second rule instead of merging the two checks into one rule. > > > sc_prohibit_readlink: > > @prohibit='\ > halt='use virFileResolveLink, not readlink' \ > > @@ -2264,6 +2269,9 @@ exclude_file_name_regexp--sc_prohibit_setuid = > > ^src/util/virutil\.c|tools/virt-l > > exclude_file_name_regexp--sc_prohibit_sprintf = \ > > ^(build-aux/syntax-check\.mk|docs/hacking\.html\.in|.*\.stp|.*\.pl)$$ > > +exclude_file_name_regexp--sc_prohibit_snprintf = \ > > + > > ^(build-aux/syntax-check\.mk|docs/hacking\.html\.in|tools/virt-login-shell\.c)$$ > > + > > Still, if we are using g_snprintf for both, a single merged rule might be > easier to use. I was considering to merge both rules, the only reason to do it this way was that we should probably forbid sprintf and printf in virt-login-shell.c as well. But I'm OK with merging it together. Pavel signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/4] syntax-check: forbid usage of snprintf
On 11/13/19 10:33 AM, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- new in v2 build-aux/syntax-check.mk | 8 1 file changed, 8 insertions(+) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index ff1198ca73..78458dce53 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -469,6 +469,11 @@ sc_prohibit_sprintf: halt='use g_snprintf, not sprintf' \ $(_sc_search_regexp) +sc_prohibit_snprintf: + @prohibit='\' \ + halt='use g_snprintf, not snprintf' \ + $(_sc_search_regexp) + Ah, you added a second rule instead of merging the two checks into one rule. sc_prohibit_readlink: @prohibit='\ +exclude_file_name_regexp--sc_prohibit_snprintf = \ + ^(build-aux/syntax-check\.mk|docs/hacking\.html\.in|tools/virt-login-shell\.c)$$ + Still, if we are using g_snprintf for both, a single merged rule might be easier to use. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/4] syntax-check: update of sprintf rule to mention g_snpritnf
On 11/13/19 10:33 AM, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- new in v2 build-aux/syntax-check.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 1c46928ac7..ff1198ca73 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -462,11 +462,11 @@ sc_prohibit_risky_id_promotion: halt='cast -1 to ([ug]id_t) before comparing against id' \ $(_sc_search_regexp) -# Use snprintf rather than s'printf, even if buffer is provably large enough, +# Use g_snprintf rather than s'printf, even if buffer is provably large enough, # since gnulib has more guarantees for snprintf portability sc_prohibit_sprintf: @prohibit='\<[s]printf\>' \ Should we be modifying @prohibit= to also prohibit bare snprintf? - halt='use snprintf, not sprintf' \ + halt='use g_snprintf, not sprintf' \ $(_sc_search_regexp) sc_prohibit_readlink: -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/4] [ACKED] bootstrap.conf: remove usage of snprintf gnulib module
Signed-off-by: Pavel Hrdina Reviewed-by: Peter Krempa --- bootstrap.conf | 1 - 1 file changed, 1 deletion(-) diff --git a/bootstrap.conf b/bootstrap.conf index 4c784487e2..747040a1a3 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -74,7 +74,6 @@ setenv setsockopt sigaction sigpipe -snprintf socket stat-time strchrnul -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/4] syntax-check: update of sprintf rule to mention g_snpritnf
Signed-off-by: Pavel Hrdina --- new in v2 build-aux/syntax-check.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 1c46928ac7..ff1198ca73 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -462,11 +462,11 @@ sc_prohibit_risky_id_promotion: halt='cast -1 to ([ug]id_t) before comparing against id' \ $(_sc_search_regexp) -# Use snprintf rather than s'printf, even if buffer is provably large enough, +# Use g_snprintf rather than s'printf, even if buffer is provably large enough, # since gnulib has more guarantees for snprintf portability sc_prohibit_sprintf: @prohibit='\<[s]printf\>' \ - halt='use snprintf, not sprintf' \ + halt='use g_snprintf, not sprintf' \ $(_sc_search_regexp) sc_prohibit_readlink: -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/4] syntax-check: forbid usage of snprintf
Signed-off-by: Pavel Hrdina --- new in v2 build-aux/syntax-check.mk | 8 1 file changed, 8 insertions(+) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index ff1198ca73..78458dce53 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -469,6 +469,11 @@ sc_prohibit_sprintf: halt='use g_snprintf, not sprintf' \ $(_sc_search_regexp) +sc_prohibit_snprintf: + @prohibit='\' \ + halt='use g_snprintf, not snprintf' \ + $(_sc_search_regexp) + sc_prohibit_readlink: @prohibit='\https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/4] replace use of gnulib snprintf by g_snprintf
Glib implementation follows the ISO C99 standard so it's safe to replace
the gnulib implementation.
Signed-off-by: Pavel Hrdina
---
changes in v2:
- don't replace snprintf in tools/virt-login-shell.c
src/hyperv/hyperv_driver.c| 4 +-
src/libxl/libxl_conf.c| 22 +--
src/libxl/libxl_migration.c | 4 +-
src/lxc/lxc_process.c | 6 +--
src/nwfilter/nwfilter_ebiptables_driver.c | 36 +-
src/nwfilter/nwfilter_learnipaddr.c | 2 +-
src/openvz/openvz_driver.c| 6 +--
src/qemu/qemu_agent.c | 46 +++
src/qemu/qemu_driver.c| 8 ++--
src/security/security_dac.c | 8 ++--
src/security/virt-aa-helper.c | 2 +-
src/storage/storage_util.c| 2 +-
src/util/virhostcpu.c | 2 +-
src/util/viriptables.c| 6 +--
src/util/virmacaddr.c | 8 ++--
src/util/virnetdevbridge.c| 4 +-
src/util/virnetdevmacvlan.c | 12 +++---
src/util/virpci.c | 4 +-
src/util/virpidfile.c | 4 +-
src/util/virtime.c| 10 ++---
src/util/virusb.c | 8 ++--
src/util/viruuid.c| 12 +++---
src/vbox/vbox_common.c| 24 ++--
src/vbox/vbox_tmpl.c | 2 +-
src/vmx/vmx.c | 18 -
src/vz/vz_driver.c| 22 +--
src/vz/vz_sdk.c | 8 ++--
tests/libxlmock.c | 2 +-
tests/qemuagenttest.c | 12 +++---
tests/testutils.c | 2 +-
tests/virnetsockettest.c | 2 +-
tests/virpcimock.c| 28 +++---
tests/virsystemdtest.c| 4 +-
tools/virsh-domain-monitor.c | 2 +-
tools/virsh-domain.c | 2 +-
tools/virt-host-validate-common.c | 2 +-
tools/vsh-table.c | 6 +--
tools/vsh.c | 22 +--
tools/wireshark/src/packet-libvirt.c | 2 +-
39 files changed, 188 insertions(+), 188 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index ac10f86b82..3a2f6602bc 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -1381,7 +1381,7 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int
codeset,
/* press the keys */
for (i = 0; i < nkeycodes; i++) {
-snprintf(keycodeStr, sizeof(keycodeStr), "%d", translatedKeycodes[i]);
+g_snprintf(keycodeStr, sizeof(keycodeStr), "%d",
translatedKeycodes[i]);
params = hypervCreateInvokeParamsList(priv, "PressKey", selector,
Msvm_Keyboard_WmiInfo);
@@ -1409,7 +1409,7 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int
codeset,
/* release the keys */
for (i = 0; i < nkeycodes; i++) {
-snprintf(keycodeStr, sizeof(keycodeStr), "%d", translatedKeycodes[i]);
+g_snprintf(keycodeStr, sizeof(keycodeStr), "%d",
translatedKeycodes[i]);
params = hypervCreateInvokeParamsList(priv, "ReleaseKey", selector,
Msvm_Keyboard_WmiInfo);
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 920f228d6a..71a503c0f7 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -419,12 +419,12 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
continue;
}
-snprintf(xlCPU,
-sizeof(xlCPU),
-"%s=0",
-xenTranslateCPUFeature(
-def->cpu->features[i].name,
-false));
+g_snprintf(xlCPU,
+ sizeof(xlCPU),
+ "%s=0",
+ xenTranslateCPUFeature(
+ def->cpu->features[i].name,
+ false));
if (libxl_cpuid_parse_config(_info->cpuid,
xlCPU)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported cpu feature '%s'"),
@@ -441,11 +441,11 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
continue;
}
-snprintf(xlCPU,
-sizeof(xlCPU),
-"%s=1",
-xenTranslateCPUFeature(
-
[libvirt] [PATCH v2 0/4] replace snprintf by g_snprintf (kill-a-gnulib-module-a-day initiative)
Pavel Hrdina (4): replace use of gnulib snprintf by g_snprintf bootstrap.conf: remove usage of snprintf gnulib module syntax-check: update of sprintf rule to mention g_snpritnf syntax-check: forbid usage of snprintf bootstrap.conf| 1 - build-aux/syntax-check.mk | 12 +- src/hyperv/hyperv_driver.c| 4 +- src/libxl/libxl_conf.c| 22 +-- src/libxl/libxl_migration.c | 4 +- src/lxc/lxc_process.c | 6 +-- src/nwfilter/nwfilter_ebiptables_driver.c | 36 +- src/nwfilter/nwfilter_learnipaddr.c | 2 +- src/openvz/openvz_driver.c| 6 +-- src/qemu/qemu_agent.c | 46 +++ src/qemu/qemu_driver.c| 8 ++-- src/security/security_dac.c | 8 ++-- src/security/virt-aa-helper.c | 2 +- src/storage/storage_util.c| 2 +- src/util/virhostcpu.c | 2 +- src/util/viriptables.c| 6 +-- src/util/virmacaddr.c | 8 ++-- src/util/virnetdevbridge.c| 4 +- src/util/virnetdevmacvlan.c | 12 +++--- src/util/virpci.c | 4 +- src/util/virpidfile.c | 4 +- src/util/virtime.c| 10 ++--- src/util/virusb.c | 8 ++-- src/util/viruuid.c| 12 +++--- src/vbox/vbox_common.c| 24 ++-- src/vbox/vbox_tmpl.c | 2 +- src/vmx/vmx.c | 18 - src/vz/vz_driver.c| 22 +-- src/vz/vz_sdk.c | 8 ++-- tests/libxlmock.c | 2 +- tests/qemuagenttest.c | 12 +++--- tests/testutils.c | 2 +- tests/virnetsockettest.c | 2 +- tests/virpcimock.c| 28 +++--- tests/virsystemdtest.c| 4 +- tools/virsh-domain-monitor.c | 2 +- tools/virsh-domain.c | 2 +- tools/virt-host-validate-common.c | 2 +- tools/vsh-table.c | 6 +-- tools/vsh.c | 22 +-- tools/wireshark/src/packet-libvirt.c | 2 +- 41 files changed, 198 insertions(+), 191 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Warn verbosely if using old loader:nvram pairs
On 11/12/19 11:17 PM, Jim Fehlig wrote: On 11/11/19 9:42 AM, Michal Privoznik wrote: There are two ways for specifying loader:nvram pairs: 1) --with-loader-nvram configure option 2) nvram variable in qemu.conf Since we have FW descriptors, using this old style is discouraged, but not as strong as one would expect. Produce more warnings: Oh, I didn't know the old style was discouraged when using FW descriptors. Thanks for mentioning it! 1) produce a warning if somebody tries the configure option 2) produce a warning if somebody sets nvram variable and at least on FW descriptor was found The reason for producing warning in case 1) is that package maintainers, who set the configure option in the first place should start moving towards FW descriptors and abandon the configure option. After all, the warning is printed into config output only in this case. Should the configure option be removed from the upstream spec file? Definitely, Fedora ships FW descriptors and so does RHEL. What's the state in OpenSUSE? But I bet you have your own spec file anyway, don't you? Just like we have for Fedora and RHEL. Will post a patch tomorrow (unless you beat me to it). Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/8] domaincaps: Store domain capability features in an array
Declare the capabilities as enum values and store them in an array. This
makes adding new features more straightforward and simplifies the
formatter which now doesn't require changing.
Signed-off-by: Peter Krempa
---
src/conf/domain_capabilities.c | 30 +++---
src/conf/domain_capabilities.h | 13 ++---
src/qemu/qemu_capabilities.c | 6 +++---
3 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 39acad00f1..1993a22cc5 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -33,6 +33,15 @@ VIR_ENUM_IMPL(virDomainCapsCPUUsable,
"unknown", "yes", "no",
);
+
+VIR_ENUM_DECL(virDomainCapsFeature);
+VIR_ENUM_IMPL(virDomainCapsFeature,
+ VIR_DOMAIN_CAPS_FEATURE_LAST,
+ "iothreads",
+ "vmcoreinfo",
+ "genid",
+);
+
static virClassPtr virDomainCapsClass;
static virClassPtr virDomainCapsCPUModelsClass;
@@ -324,9 +333,10 @@ virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum)
void
virDomainCapsFeaturesInitUnsupported(virDomainCapsPtr caps)
{
-caps->iothreads = VIR_TRISTATE_BOOL_NO;
-caps->vmcoreinfo = VIR_TRISTATE_BOOL_NO;
-caps->genid = VIR_TRISTATE_BOOL_NO;
+size_t i;
+
+for (i = 0; i < VIR_DOMAIN_CAPS_FEATURE_LAST; i++)
+caps->features[i] = VIR_TRISTATE_BOOL_NO;
}
@@ -619,11 +629,16 @@ virDomainCapsFormatFeatures(const virDomainCaps *caps,
virBufferPtr buf)
{
g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
+size_t i;
virDomainCapsFeatureGICFormat(, >gic);
-qemuDomainCapsFeatureFormatSimple(, "iothreads", caps->iothreads);
-qemuDomainCapsFeatureFormatSimple(, "vmcoreinfo",
caps->vmcoreinfo);
-qemuDomainCapsFeatureFormatSimple(, "genid", caps->genid);
+
+for (i = 0; i < VIR_DOMAIN_CAPS_FEATURE_LAST; i++) {
+qemuDomainCapsFeatureFormatSimple(,
+ virDomainCapsFeatureTypeToString(i),
+ caps->features[i]);
+}
+
virDomainCapsFeatureSEVFormat(, caps->sev);
virXMLFormatElement(buf, "features", NULL, );
@@ -649,7 +664,8 @@ virDomainCapsFormat(const virDomainCaps *caps)
if (caps->maxvcpus)
virBufferAsprintf(, "\n", caps->maxvcpus);
-qemuDomainCapsFeatureFormatSimple(, "iothreads", caps->iothreads);
+qemuDomainCapsFeatureFormatSimple(, "iothreads",
+
caps->features[VIR_DOMAIN_CAPS_FEATURE_IOTHREADS]);
virDomainCapsOSFormat(, >os);
virDomainCapsCPUFormat(, >cpu);
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 9baaea8f60..83a9713a8b 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -156,6 +156,14 @@ struct _virSEVCapability {
unsigned int reduced_phys_bits;
};
+typedef enum {
+VIR_DOMAIN_CAPS_FEATURE_IOTHREADS = 0,
+VIR_DOMAIN_CAPS_FEATURE_VMCOREINFO,
+VIR_DOMAIN_CAPS_FEATURE_GENID,
+
+VIR_DOMAIN_CAPS_FEATURE_LAST
+} virDomainCapsFeature;
+
struct _virDomainCaps {
virObjectLockable parent;
@@ -166,7 +174,6 @@ struct _virDomainCaps {
/* Some machine specific info */
int maxvcpus;
-virTristateBool iothreads; /* Whether I/O threads are supported or not. */
virDomainCapsOS os;
virDomainCapsCPU cpu;
@@ -178,10 +185,10 @@ struct _virDomainCaps {
/* add new domain devices here */
virDomainCapsFeatureGIC gic;
-virTristateBool vmcoreinfo;
-virTristateBool genid;
virSEVCapabilityPtr sev;
/* add new domain features here */
+
+virTristateBool features[VIR_DOMAIN_CAPS_FEATURE_LAST];
};
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainCaps, virObjectUnref);
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 23a358bf91..a1fbf0da34 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5285,7 +5285,7 @@ static void
virQEMUCapsFillDomainIOThreadCaps(virQEMUCapsPtr qemuCaps,
virDomainCapsPtr domCaps)
{
-domCaps->iothreads = virTristateBoolFromBool(
+domCaps->features[VIR_DOMAIN_CAPS_FEATURE_IOTHREADS] =
virTristateBoolFromBool(
virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD));
}
@@ -5584,10 +5584,10 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus);
}
-domCaps->vmcoreinfo = virTristateBoolFromBool(
+domCaps->features[VIR_DOMAIN_CAPS_FEATURE_VMCOREINFO] =
virTristateBoolFromBool(
virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMCOREINFO));
-domCaps->genid = virTristateBoolFromBool(
+domCaps->features[VIR_DOMAIN_CAPS_FEATURE_GENID] = virTristateBoolFromBool(
virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMGENID));
if (virQEMUCapsFillDomainOSCaps(os,
--
2.23.0
--
libvir-list
[libvirt] [PATCH 2/8] conf: domaincaps: Extract formatting of the subelement
Extract it to virDomainCapsFormatFeatures so that the main function does
not get so bloated over time.
Signed-off-by: Peter Krempa
---
src/conf/domain_capabilities.c | 28 ++--
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 099963dc6a..2c1c5fc9e8 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -600,6 +600,23 @@ virDomainCapsFeatureSEVFormat(virBufferPtr buf,
}
+static void
+virDomainCapsFormatFeatures(const virDomainCaps *caps,
+virBufferPtr buf)
+{
+virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(buf, 2);
+
+virDomainCapsFeatureGICFormat(buf, >gic);
+qemuDomainCapsFeatureFormatSimple(buf, "vmcoreinfo", caps->vmcoreinfo);
+qemuDomainCapsFeatureFormatSimple(buf, "genid", caps->genid);
+virDomainCapsFeatureSEVFormat(buf, caps->sev);
+
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+}
+
+
char *
virDomainCapsFormat(const virDomainCaps *caps)
{
@@ -636,16 +653,7 @@ virDomainCapsFormat(const virDomainCaps *caps)
virBufferAdjustIndent(, -2);
virBufferAddLit(, "\n");
-virBufferAddLit(, "\n");
-virBufferAdjustIndent(, 2);
-
-virDomainCapsFeatureGICFormat(, >gic);
-qemuDomainCapsFeatureFormatSimple(, "vmcoreinfo", caps->vmcoreinfo);
-qemuDomainCapsFeatureFormatSimple(, "genid", caps->genid);
-virDomainCapsFeatureSEVFormat(, caps->sev);
-
-virBufferAdjustIndent(, -2);
-virBufferAddLit(, "\n");
+virDomainCapsFormatFeatures(caps, );
virBufferAdjustIndent(, -2);
virBufferAddLit(, "\n");
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/8] qemu: domcaps: Initialize all features
While the qemu driver currently implements all domain capability
features, we should initialize all features using the helper similarly
to how we do it in drivers which don't support any.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_capabilities.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b03abcd0d3..23a358bf91 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5571,6 +5571,8 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
virDomainCapsDeviceVideoPtr video = >video;
virDomainCapsDeviceRNGPtr rng = >rng;
+virDomainCapsFeaturesInitUnsupported(domCaps);
+
domCaps->maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps,
domCaps->machine);
if (domCaps->virttype == VIR_DOMAIN_VIRT_KVM) {
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] util: use g_vsnprintf
On Wed, Nov 13, 2019 at 10:33:08 +0100, Ján Tomko wrote: > Instead of vsnprintf from gnulib, use g_vsnprintf from GLib. > > Signed-off-by: Ján Tomko > --- > src/util/virerror.c | 4 ++-- > src/util/virtypedparam.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) I think you should add a syntax-check rule forbidding use of plain vsnprintf. With that: Reviewed-by: Peter Krempa I queued the appropriate patch to bootstrap.conf to my branch of the kill-a-gnulib-module-a-day initiative. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/8] qemu: domcaps: Simplify adding new domaincaps based on qemu caps
Add a helper which converts qemu emulator capabilities to the domain
capability XML. This will simplify future additions of new features.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_capabilities.c | 39 ++--
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a1fbf0da34..483c3fcf0f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5281,12 +5281,35 @@ virQEMUCapsFillDomainCPUCaps(virCapsPtr caps,
}
+struct virQEMUCapsDomainFeatureCapabilityTuple {
+virDomainCapsFeature domcap;
+virQEMUCapsFlags qemucap;
+};
+
+/**
+ * This maps the qemu features to the entries in of the domain
+ * capability XML. Use QEMU_CAPS_LAST as qemucap to always enable the feature.
+ */
+static const struct virQEMUCapsDomainFeatureCapabilityTuple domCapsTuples[] = {
+{ VIR_DOMAIN_CAPS_FEATURE_IOTHREADS, QEMU_CAPS_OBJECT_IOTHREAD },
+{ VIR_DOMAIN_CAPS_FEATURE_VMCOREINFO, QEMU_CAPS_DEVICE_VMCOREINFO },
+{ VIR_DOMAIN_CAPS_FEATURE_GENID, QEMU_CAPS_DEVICE_VMGENID },
+};
+
+
static void
-virQEMUCapsFillDomainIOThreadCaps(virQEMUCapsPtr qemuCaps,
- virDomainCapsPtr domCaps)
+virQEMUCapsFillDomainFeaturesFromQEMUCaps(virQEMUCapsPtr qemuCaps,
+ virDomainCapsPtr domCaps)
{
-domCaps->features[VIR_DOMAIN_CAPS_FEATURE_IOTHREADS] =
virTristateBoolFromBool(
-virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD));
+size_t i;
+
+virDomainCapsFeaturesInitUnsupported(domCaps);
+
+for (i = 0; i < G_N_ELEMENTS(domCapsTuples); i++) {
+if (domCapsTuples[i].qemucap == QEMU_CAPS_LAST ||
+virQEMUCapsGet(qemuCaps, domCapsTuples[i].qemucap))
+domCaps->features[domCapsTuples[i].domcap] = VIR_TRISTATE_BOOL_YES;
+}
}
@@ -5572,6 +5595,7 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
virDomainCapsDeviceRNGPtr rng = >rng;
virDomainCapsFeaturesInitUnsupported(domCaps);
+virQEMUCapsFillDomainFeaturesFromQEMUCaps(qemuCaps, domCaps);
domCaps->maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps,
domCaps->machine);
@@ -5584,12 +5608,6 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus);
}
-domCaps->features[VIR_DOMAIN_CAPS_FEATURE_VMCOREINFO] =
virTristateBoolFromBool(
-virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMCOREINFO));
-
-domCaps->features[VIR_DOMAIN_CAPS_FEATURE_GENID] = virTristateBoolFromBool(
-virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMGENID));
-
if (virQEMUCapsFillDomainOSCaps(os,
domCaps->machine,
domCaps->arch,
@@ -5598,7 +5616,6 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
return -1;
virQEMUCapsFillDomainCPUCaps(caps, qemuCaps, domCaps);
-virQEMUCapsFillDomainIOThreadCaps(qemuCaps, domCaps);
virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, domCaps->machine, disk);
virQEMUCapsFillDomainDeviceGraphicsCaps(qemuCaps, graphics);
virQEMUCapsFillDomainDeviceVideoCaps(qemuCaps, video);
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/8] conf: domaincaps: Replace FORMAT_SINGLE macro by a function
Introduce qemuDomainCapsFeatureFormatSimple which does exactly the same
thing but it's a function.
Signed-off-by: Peter Krempa
---
src/conf/domain_capabilities.c | 27 ---
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index f922cb9055..099963dc6a 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -380,14 +380,6 @@ virDomainCapsStringValuesFormat(virBufferPtr buf,
virBufferAddLit(buf, "\n"); \
} while (0)
-#define FORMAT_SINGLE(name, supported) \
-do { \
-if (supported != VIR_TRISTATE_BOOL_ABSENT) { \
-virBufferAsprintf(, "<%s supported='%s'/>\n", name, \
-(supported == VIR_TRISTATE_BOOL_YES) ? "yes" : "no"); \
-} \
-} while (0)
-
#define ENUM_PROCESS(master, capsEnum, valToStr) \
do { \
virDomainCapsEnumFormat(buf, >capsEnum, \
@@ -395,6 +387,19 @@ virDomainCapsStringValuesFormat(virBufferPtr buf,
} while (0)
+static void
+qemuDomainCapsFeatureFormatSimple(virBufferPtr buf,
+ const char *featurename,
+ virTristateBool supported)
+{
+if (supported == VIR_TRISTATE_BOOL_ABSENT)
+return;
+
+virBufferAsprintf(buf, "<%s supported='%s'/>\n", featurename,
+ virTristateBoolTypeToString(supported));
+}
+
+
static void
virDomainCapsLoaderFormat(virBufferPtr buf,
const virDomainCapsLoader *loader)
@@ -614,7 +619,7 @@ virDomainCapsFormat(const virDomainCaps *caps)
if (caps->maxvcpus)
virBufferAsprintf(, "\n", caps->maxvcpus);
-FORMAT_SINGLE("iothreads", caps->iothreads);
+qemuDomainCapsFeatureFormatSimple(, "iothreads", caps->iothreads);
virDomainCapsOSFormat(, >os);
virDomainCapsCPUFormat(, >cpu);
@@ -635,8 +640,8 @@ virDomainCapsFormat(const virDomainCaps *caps)
virBufferAdjustIndent(, 2);
virDomainCapsFeatureGICFormat(, >gic);
-FORMAT_SINGLE("vmcoreinfo", caps->vmcoreinfo);
-FORMAT_SINGLE("genid", caps->genid);
+qemuDomainCapsFeatureFormatSimple(, "vmcoreinfo", caps->vmcoreinfo);
+qemuDomainCapsFeatureFormatSimple(, "genid", caps->genid);
virDomainCapsFeatureSEVFormat(, caps->sev);
virBufferAdjustIndent(, -2);
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: use GUINT32_SWAP_LE_BE
On Wed, Nov 13, 2019 at 10:38:06 +, Daniel Berrange wrote:
> On Wed, Nov 13, 2019 at 10:33:09AM +0100, Ján Tomko wrote:
> > Use this GLib macro instead of bswap_32 from gnulib.
> >
> > Signed-off-by: Ján Tomko
> > ---
> > src/qemu/qemu_driver.c | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
>
> Missing bootstrap.conf change to remove byteswap
I queued this to my branch for bootstrap.conf changes.
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index c969a3d463..85b0c9cb79 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -2803,11 +2803,11 @@ struct _virQEMUSaveData {
> > static inline void
> > bswap_header(virQEMUSaveHeaderPtr hdr)
> > {
> > -hdr->version = bswap_32(hdr->version);
> > -hdr->data_len = bswap_32(hdr->data_len);
> > -hdr->was_running = bswap_32(hdr->was_running);
> > -hdr->compressed = bswap_32(hdr->compressed);
> > -hdr->cookieOffset = bswap_32(hdr->cookieOffset);
> > +hdr->version = GUINT32_SWAP_LE_BE(hdr->version);
> > +hdr->data_len = GUINT32_SWAP_LE_BE(hdr->data_len);
> > +hdr->was_running = GUINT32_SWAP_LE_BE(hdr->was_running);
> > +hdr->compressed = GUINT32_SWAP_LE_BE(hdr->compressed);
> > +hdr->cookieOffset = GUINT32_SWAP_LE_BE(hdr->cookieOffset);
> > }
>
> Also needs to remove byteswap.h include file
.. so with the above addressed:
Reviewed-by: Peter Krempa
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/8] conf: domaincaps: Use virXMLFormatElement in virDomainCapsFormatFeatures
Signed-off-by: Peter Krempa
---
src/conf/domain_capabilities.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 2c1c5fc9e8..d9a9093f89 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -604,16 +604,14 @@ static void
virDomainCapsFormatFeatures(const virDomainCaps *caps,
virBufferPtr buf)
{
-virBufferAddLit(buf, "\n");
-virBufferAdjustIndent(buf, 2);
+g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
-virDomainCapsFeatureGICFormat(buf, >gic);
-qemuDomainCapsFeatureFormatSimple(buf, "vmcoreinfo", caps->vmcoreinfo);
-qemuDomainCapsFeatureFormatSimple(buf, "genid", caps->genid);
-virDomainCapsFeatureSEVFormat(buf, caps->sev);
+virDomainCapsFeatureGICFormat(, >gic);
+qemuDomainCapsFeatureFormatSimple(, "vmcoreinfo",
caps->vmcoreinfo);
+qemuDomainCapsFeatureFormatSimple(, "genid", caps->genid);
+virDomainCapsFeatureSEVFormat(, caps->sev);
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
+virXMLFormatElement(buf, "features", NULL, );
}
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/8] domcaps: Add function for initializing domain caps as unsupported
For future extensions of the domain caps it's useful to have a single
point that initializes all capabilities as unsupported by a driver. The
driver then can enable specific ones.
Signed-off-by: Peter Krempa
---
src/bhyve/bhyve_capabilities.c | 4 +---
src/conf/domain_capabilities.c | 14 ++
src/conf/domain_capabilities.h | 2 ++
src/libvirt_private.syms | 1 +
src/libxl/libxl_capabilities.c | 5 ++---
5 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c
index c04a475375..f80cf7be62 100644
--- a/src/bhyve/bhyve_capabilities.c
+++ b/src/bhyve/bhyve_capabilities.c
@@ -116,9 +116,7 @@ virBhyveDomainCapsFill(virDomainCapsPtr caps,
}
caps->hostdev.supported = VIR_TRISTATE_BOOL_NO;
-caps->iothreads = VIR_TRISTATE_BOOL_NO;
-caps->vmcoreinfo = VIR_TRISTATE_BOOL_NO;
-caps->genid = VIR_TRISTATE_BOOL_NO;
+virDomainCapsFeaturesInitUnsupported(caps);
caps->gic.supported = VIR_TRISTATE_BOOL_NO;
return 0;
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 8d0a0c121c..39acad00f1 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -316,6 +316,20 @@ virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum)
}
+/**
+ * @caps: domain caps
+ *
+ * Initializes all features in 'caps' as unsupported.
+ */
+void
+virDomainCapsFeaturesInitUnsupported(virDomainCapsPtr caps)
+{
+caps->iothreads = VIR_TRISTATE_BOOL_NO;
+caps->vmcoreinfo = VIR_TRISTATE_BOOL_NO;
+caps->genid = VIR_TRISTATE_BOOL_NO;
+}
+
+
static int
virDomainCapsEnumFormat(virBufferPtr buf,
const virDomainCapsEnum *capsEnum,
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 6b27eac11f..9baaea8f60 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -226,6 +226,8 @@ int virDomainCapsEnumSet(virDomainCapsEnumPtr capsEnum,
unsigned int *values);
void virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum);
+void virDomainCapsFeaturesInitUnsupported(virDomainCapsPtr caps);
+
char * virDomainCapsFormat(const virDomainCaps *caps);
int virDomainCapsDeviceDefValidate(const virDomainCaps *caps,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4d0d03c580..1432f1697a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -202,6 +202,7 @@ virDomainCapsCPUUsableTypeToString;
virDomainCapsDeviceDefValidate;
virDomainCapsEnumClear;
virDomainCapsEnumSet;
+virDomainCapsFeaturesInitUnsupported;
virDomainCapsFormat;
virDomainCapsNew;
virSEVCapabilitiesFree;
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index fe792e9a82..55f6b490ec 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -764,9 +764,8 @@ libxlMakeDomainCapabilities(virDomainCapsPtr domCaps,
libxlMakeDomainDeviceHostdevCaps(hostdev) < 0)
return -1;
-domCaps->iothreads = VIR_TRISTATE_BOOL_NO;
-domCaps->vmcoreinfo = VIR_TRISTATE_BOOL_NO;
-domCaps->genid = VIR_TRISTATE_BOOL_NO;
+virDomainCapsFeaturesInitUnsupported(domCaps);
+
domCaps->gic.supported = VIR_TRISTATE_BOOL_NO;
return 0;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/8] domaincaps: Simplify adding of new domain capability features
As you might guess I plan to add a new one later on. Peter Krempa (8): conf: domaincaps: Replace FORMAT_SINGLE macro by a function conf: domaincaps: Extract formatting of the subelement conf: domaincaps: Use virXMLFormatElement in virDomainCapsFormatFeatures conf: domaincaps: Add 'iothreads' to the features section domcaps: Add function for initializing domain caps as unsupported qemu: domcaps: Initialize all features domaincaps: Store domain capability features in an array qemu: domcaps: Simplify adding new domaincaps based on qemu caps docs/schemas/domaincaps.rng | 3 + src/bhyve/bhyve_capabilities.c| 4 +- src/conf/domain_capabilities.c| 80 ++- src/conf/domain_capabilities.h| 15 +++- src/libvirt_private.syms | 1 + src/libxl/libxl_capabilities.c| 5 +- src/qemu/qemu_capabilities.c | 41 +++--- tests/domaincapsdata/libxl-xenfv.xml | 1 + tests/domaincapsdata/libxl-xenpv.xml | 1 + .../domaincapsdata/qemu_1.5.3-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_1.5.3-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_1.5.3.x86_64.xml| 1 + .../domaincapsdata/qemu_1.6.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_1.6.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_1.6.0.x86_64.xml| 1 + .../domaincapsdata/qemu_1.7.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_1.7.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_1.7.0.x86_64.xml| 1 + .../domaincapsdata/qemu_2.1.1-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.1.1-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.1.1.x86_64.xml| 1 + .../domaincapsdata/qemu_2.10.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.10.0-tcg.x86_64.xml | 1 + .../qemu_2.10.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.10.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.10.0.ppc64.xml| 1 + tests/domaincapsdata/qemu_2.10.0.s390x.xml| 1 + tests/domaincapsdata/qemu_2.10.0.x86_64.xml | 1 + .../domaincapsdata/qemu_2.11.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.11.0.s390x.xml| 1 + tests/domaincapsdata/qemu_2.11.0.x86_64.xml | 1 + .../domaincapsdata/qemu_2.12.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 1 + .../qemu_2.12.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.12.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.12.0.ppc64.xml| 1 + tests/domaincapsdata/qemu_2.12.0.s390x.xml| 1 + tests/domaincapsdata/qemu_2.12.0.x86_64.xml | 1 + .../domaincapsdata/qemu_2.4.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.4.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.4.0.x86_64.xml| 1 + .../domaincapsdata/qemu_2.5.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.5.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.5.0.x86_64.xml| 1 + .../domaincapsdata/qemu_2.6.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.6.0-tcg.x86_64.xml | 1 + .../qemu_2.6.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.6.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.6.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_2.6.0.x86_64.xml| 1 + .../domaincapsdata/qemu_2.7.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.7.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.7.0.s390x.xml | 1 + tests/domaincapsdata/qemu_2.7.0.x86_64.xml| 1 + .../domaincapsdata/qemu_2.8.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.8.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.8.0.s390x.xml | 1 + tests/domaincapsdata/qemu_2.8.0.x86_64.xml| 1 + .../domaincapsdata/qemu_2.9.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.9.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.9.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_2.9.0.s390x.xml | 1 + tests/domaincapsdata/qemu_2.9.0.x86_64.xml| 1 + .../domaincapsdata/qemu_3.0.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_3.0.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_3.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_3.0.0.s390x.xml | 1 + tests/domaincapsdata/qemu_3.0.0.x86_64.xml| 1 + .../domaincapsdata/qemu_3.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_3.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_3.1.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_3.1.0.x86_64.xml| 1 + .../domaincapsdata/qemu_4.0.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_4.0.0-tcg.x86_64.xml | 1 + .../qemu_4.0.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.0.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_4.0.0.s390x.xml | 1 + tests/domaincapsdata/qemu_4.0.0.x86_64.xml| 1 + .../domaincapsdata/qemu_4.1.0-q35.x86_64.xml | 1 +
[libvirt] [PATCH 4/8] conf: domaincaps: Add 'iothreads' to the features section
Historically iothreads were the first feature and thus didn't have it's own section. Add them to for consistency with other features. Unfortunately we must keep the original one in place. Signed-off-by: Peter Krempa --- docs/schemas/domaincaps.rng | 3 +++ src/conf/domain_capabilities.c| 1 + tests/domaincapsdata/libxl-xenfv.xml | 1 + tests/domaincapsdata/libxl-xenpv.xml | 1 + tests/domaincapsdata/qemu_1.5.3-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_1.5.3-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_1.5.3.x86_64.xml| 1 + tests/domaincapsdata/qemu_1.6.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_1.6.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_1.6.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_1.7.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_1.7.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_1.7.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.1.1-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.1.1-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.1.1.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.10.0-q35.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.10.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.10.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.10.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.10.0.ppc64.xml| 1 + tests/domaincapsdata/qemu_2.10.0.s390x.xml| 1 + tests/domaincapsdata/qemu_2.10.0.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.11.0-q35.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.11.0.s390x.xml| 1 + tests/domaincapsdata/qemu_2.11.0.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.12.0-q35.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.12.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.12.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.12.0.ppc64.xml| 1 + tests/domaincapsdata/qemu_2.12.0.s390x.xml| 1 + tests/domaincapsdata/qemu_2.12.0.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.4.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.4.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.4.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.5.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.5.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.5.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.6.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.6.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.6.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.6.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.6.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_2.6.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.7.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.7.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.7.0.s390x.xml | 1 + tests/domaincapsdata/qemu_2.7.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.8.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.8.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.8.0.s390x.xml | 1 + tests/domaincapsdata/qemu_2.8.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.9.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.9.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_2.9.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_2.9.0.s390x.xml | 1 + tests/domaincapsdata/qemu_2.9.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_3.0.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_3.0.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_3.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_3.0.0.s390x.xml | 1 + tests/domaincapsdata/qemu_3.0.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_3.1.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_3.1.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_3.1.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_3.1.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.0.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.0.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.0.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.0.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_4.0.0.s390x.xml | 1 + tests/domaincapsdata/qemu_4.0.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.1.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.1.0.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml| 1 + tests/domaincapsdata/qemu_4.2.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.2.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.2.0.ppc64.xml | 1 +
[libvirt] [PATCH 07/12] storage: use GRegex virStorageBackendLogicalParseVolExtents
Using GRegex simplifies the code since g_match_info_fetch will
copy the matched substring for us.
Signed-off-by: Ján Tomko
---
src/storage/storage_backend_logical.c | 49 +++
1 file changed, 13 insertions(+), 36 deletions(-)
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index 48023abd1a..d6c81e25b1 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -23,7 +23,6 @@
#include
#include
-#include
#include
#include
@@ -121,16 +120,16 @@ static int
virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol,
char **const groups)
{
+g_autoptr(GRegex) re = NULL;
+g_autoptr(GError) err = NULL;
+g_autoptr(GMatchInfo) info = NULL;
int nextents, ret = -1;
const char *regex_unit = "(\\S+)\\((\\S+)\\)";
char *p = NULL;
size_t i;
-int err, nvars;
unsigned long long offset, size, length;
virStorageVolSourceExtent extent;
g_autofree char *regex = NULL;
-g_autofree regex_t *reg = NULL;
-g_autofree regmatch_t *vars = NULL;
memset(, 0, sizeof(extent));
@@ -174,29 +173,14 @@
virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol,
strcat(regex, regex_unit);
}
-if (VIR_ALLOC(reg) < 0)
-goto cleanup;
-
-/* Each extent has a "path:offset" pair, and vars[0] will
- * be the whole matched string.
- */
-nvars = (nextents * 2) + 1;
-if (VIR_ALLOC_N(vars, nvars) < 0)
-goto cleanup;
-
-err = regcomp(reg, regex, REG_EXTENDED);
-if (err != 0) {
-char error[100];
-regerror(err, reg, error, sizeof(error));
+re = g_regex_new(regex, 0, 0, );
+if (!re) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to compile regex %s"),
- error);
-goto cleanup;
+ _("Failed to compile regex %s"), err->message);
+return -1;
}
-err = regexec(reg, groups[3], nvars, vars, 0);
-regfree(reg);
-if (err != 0) {
+if (!g_regex_match(re, groups[3], 0, )) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("malformed volume extent devices value"));
goto cleanup;
@@ -204,23 +188,16 @@
virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol,
p = groups[3];
-/* vars[0] is skipped */
+/* Each extent has a "path:offset" pair, and match #0
+ * is the whole matched string.
+ */
for (i = 0; i < nextents; i++) {
size_t j;
-int len;
g_autofree char *offset_str = NULL;
j = (i * 2) + 1;
-len = vars[j].rm_eo - vars[j].rm_so;
-p[vars[j].rm_eo] = '\0';
-
-if (VIR_STRNDUP(extent.path,
-p + vars[j].rm_so, len) < 0)
-goto cleanup;
-
-len = vars[j + 1].rm_eo - vars[j + 1].rm_so;
-if (VIR_STRNDUP(offset_str, p + vars[j + 1].rm_so, len) < 0)
-goto cleanup;
+extent.path = g_match_info_fetch(info, j);
+offset_str = g_match_info_fetch(info, j + 1);
if (virStrToLong_ull(offset_str, NULL, 10, ) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/12] libxl: use g_autofree in xenParseSxprVifRate
Signed-off-by: Ján Tomko
---
src/libxl/xen_common.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index c31a5c952e..c06ab6e995 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -1060,7 +1060,7 @@ static const char *vif_bytes_per_sec_re =
"^[0-9]+[GMK]?[Bb]/s$";
static int
xenParseSxprVifRate(const char *rate, unsigned long long *kbytes_per_sec)
{
-char *trate = NULL;
+g_autofree char *trate = NULL;
char *p;
regex_t rec;
int err;
@@ -1109,7 +1109,6 @@ xenParseSxprVifRate(const char *rate, unsigned long long
*kbytes_per_sec)
cleanup:
regfree();
-VIR_FREE(trate);
return ret;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/12] libxl: do not use G_REGEX_EXTENDED
This flag is not needed to use extended regular expression syntax
with GRegex and it makes GRegex ignore whitespace in the regex.
Remove the unintended usage, even though it should not matter in this
case.
Signed-off-by: Ján Tomko
---
src/libxl/libxl_capabilities.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index fe792e9a82..0fb9c0b344 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -397,7 +397,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
return -1;
}
-regex = g_regex_new(XEN_CAP_REGEX, G_REGEX_EXTENDED, 0, );
+regex = g_regex_new(XEN_CAP_REGEX, 0, 0, );
if (!regex) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to compile regex %s"), err->message);
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] replace use of gnulib snprintf by g_snprintf
On Wed, Nov 13, 2019 at 15:00:31 +0100, Pavel Hrdina wrote: > Glib implementation follows the ISO C99 standard so it's safe to replace > the gnulib implementation. > > Signed-off-by: Pavel Hrdina > --- Please add a syntax-check rule forbidding use of snprintf. Reviewed-by: Peter Krempa -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] bootstrap.conf: remove usage of snprintf gnulib module
On Wed, Nov 13, 2019 at 15:00:32 +0100, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina > --- > bootstrap.conf | 1 - > 1 file changed, 1 deletion(-) Thanks for participating in this initiative! I'll queue this with my R-b applied to my branch. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/12] libxl: remove 'ret' from xenParseSxprVifRate
Now that the cleanup section is empty, the ret variable is no longer
necessary.
Signed-off-by: Ján Tomko
---
src/libxl/xen_common.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index 15a2db8add..0cb7f0f331 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -1064,7 +1064,6 @@ xenParseSxprVifRate(const char *rate, unsigned long long
*kbytes_per_sec)
char *p;
char *suffix;
unsigned long long tmp;
-int ret = -1;
trate = g_strdup(rate);
@@ -1082,13 +1081,13 @@ xenParseSxprVifRate(const char *rate, unsigned long
long *kbytes_per_sec)
if (!g_regex_match(regex, trate, 0, NULL)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid rate '%s' specified"), rate);
-goto cleanup;
+return -1;
}
if (virStrToLong_ull(rate, , 10, )) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to parse rate '%s'"), rate);
-goto cleanup;
+return -1;
}
if (*suffix == 'G')
@@ -1100,10 +1099,7 @@ xenParseSxprVifRate(const char *rate, unsigned long long
*kbytes_per_sec)
tmp /= 8;
*kbytes_per_sec = tmp;
-ret = 0;
-
- cleanup:
-return ret;
+return 0;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 12/12] tests: use GRegex in vboxsnapshotxmltest
Signed-off-by: Ján Tomko
---
tests/vboxsnapshotxmltest.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/tests/vboxsnapshotxmltest.c b/tests/vboxsnapshotxmltest.c
index 8faf2b9c26..d1a7522931 100644
--- a/tests/vboxsnapshotxmltest.c
+++ b/tests/vboxsnapshotxmltest.c
@@ -4,7 +4,6 @@
#ifdef WITH_VBOX
-# include
# include "vbox/vbox_snapshot_conf.h"
# define VIR_FROM_THIS VIR_FROM_NONE
@@ -12,7 +11,7 @@
static const char *testSnapshotXMLVariableLineRegexStr =
"lastStateChange=[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z";
-regex_t *testSnapshotXMLVariableLineRegex = NULL;
+GRegex *testSnapshotXMLVariableLineRegex = NULL;
static char *
testFilterXML(char *xml)
@@ -29,8 +28,7 @@ testFilterXML(char *xml)
VIR_FREE(xml);
for (xmlLine = xmlLines; *xmlLine; xmlLine++) {
-if (regexec(testSnapshotXMLVariableLineRegex,
-*xmlLine, 0, NULL, 0) == 0)
+if (g_regex_match(testSnapshotXMLVariableLineRegex, *xmlLine, 0, NULL))
continue;
virBufferStrcat(, *xmlLine, "\n", NULL);
@@ -112,12 +110,12 @@ static int
mymain(void)
{
int ret = 0;
-if (VIR_ALLOC(testSnapshotXMLVariableLineRegex) < 0)
-goto cleanup;
+g_autoptr(GError) err = NULL;
+
+testSnapshotXMLVariableLineRegex =
g_regex_new(testSnapshotXMLVariableLineRegexStr,
+ 0, 0, );
-if (regcomp(testSnapshotXMLVariableLineRegex,
-testSnapshotXMLVariableLineRegexStr,
-REG_EXTENDED | REG_NOSUB) != 0) {
+if (!testSnapshotXMLVariableLineRegex) {
ret = -1;
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
"failed to compile test regex");
@@ -136,9 +134,7 @@ mymain(void)
DO_TEST("2disks-3snap-brother");
cleanup:
-if (testSnapshotXMLVariableLineRegex)
-regfree(testSnapshotXMLVariableLineRegex);
-VIR_FREE(testSnapshotXMLVariableLineRegex);
+g_regex_unref(testSnapshotXMLVariableLineRegex);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/12] util: use GRegex in virCommandRunRegex
This saves us from allocating vars upfront, since GLib deals with
that for us.
Signed-off-by: Ján Tomko
---
src/util/vircommand.c | 37 +
1 file changed, 13 insertions(+), 24 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 49432ddfcb..1ab3dbc819 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -22,7 +22,6 @@
#include
#include
-#include
#include
#include
#include
@@ -3077,11 +3076,10 @@ virCommandRunRegex(virCommandPtr cmd,
const char *prefix,
int *exitstatus)
{
-int err;
-regex_t *reg;
-g_autofree regmatch_t *vars = NULL;
+GRegex **reg = NULL;
+g_autoptr(GMatchInfo) info = NULL;
size_t i, j, k;
-int totgroups = 0, ngroup = 0, maxvars = 0;
+int totgroups = 0, ngroup = 0;
char **groups;
g_autofree char *outbuf = NULL;
VIR_AUTOSTRINGLIST lines = NULL;
@@ -3092,29 +3090,23 @@ virCommandRunRegex(virCommandPtr cmd,
return -1;
for (i = 0; i < nregex; i++) {
-err = regcomp([i], regex[i], REG_EXTENDED);
-if (err != 0) {
-char error[100];
-regerror(err, [i], error, sizeof(error));
+g_autoptr(GError) err = NULL;
+reg[i] = g_regex_new(regex[i], G_REGEX_OPTIMIZE, 0, );
+if (!reg[i]) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to compile regex %s"), error);
+ _("Failed to compile regex %s"), err->message);
for (j = 0; j < i; j++)
-regfree([j]);
+g_regex_unref(reg[j]);
VIR_FREE(reg);
return -1;
}
totgroups += nvars[i];
-if (nvars[i] > maxvars)
-maxvars = nvars[i];
-
}
/* Storage for matched variables */
if (VIR_ALLOC_N(groups, totgroups) < 0)
goto cleanup;
-if (VIR_ALLOC_N(vars, maxvars+1) < 0)
-goto cleanup;
virCommandSetOutputBuffer(cmd, );
if (virCommandRun(cmd, exitstatus) < 0)
@@ -3140,15 +3132,12 @@ virCommandRunRegex(virCommandPtr cmd,
ngroup = 0;
for (i = 0; i < nregex; i++) {
-if (regexec([i], p, nvars[i]+1, vars, 0) != 0)
+if (!(g_regex_match(reg[i], p, 0, )))
break;
-/* NB vars[0] is the full pattern, so we offset j by 1 */
-for (j = 1; j <= nvars[i]; j++) {
-if (VIR_STRNDUP(groups[ngroup++], p + vars[j].rm_so,
-vars[j].rm_eo - vars[j].rm_so) < 0)
-goto cleanup;
-}
+/* NB match #0 is the full pattern, so we offset j by 1 */
+for (j = 1; j <= nvars[i]; j++)
+groups[ngroup++] = g_match_info_fetch(info, j);
}
/* We've matched on the last regex, so callback time */
@@ -3170,7 +3159,7 @@ virCommandRunRegex(virCommandPtr cmd,
}
for (i = 0; i < nregex; i++)
-regfree([i]);
+g_regex_unref(reg[i]);
VIR_FREE(reg);
return ret;
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/12] Use GRegex instead of regcomp (glib chronicles)
This removes the need to use gnulib's regex module. Ján Tomko (12): libxl: do not use G_REGEX_EXTENDED remove unused regex.h includes libxl: use GRegex in libxlGetAutoballoonConf libxl: use g_autofree in xenParseSxprVifRate libxl: use GRegex in xenParseSxprVifRate libxl: remove 'ret' from xenParseSxprVifRate storage: use GRegex virStorageBackendLogicalParseVolExtents util: use GRegex in virCommandRunRegex util: use GRegex for virLogRegex util: use GRegex in virStringSearch util: use GRegex in virStringMatch tests: use GRegex in vboxsnapshotxmltest src/libxl/libxl_capabilities.c| 2 +- src/libxl/libxl_conf.c| 20 -- src/libxl/libxl_driver.c | 1 - src/libxl/xen_common.c| 33 ++--- src/storage/storage_backend_logical.c | 49 +++-- src/storage/storage_util.c| 1 - src/util/vircommand.c | 37 +++ src/util/viriscsi.c | 2 - src/util/virlog.c | 15 +++- src/util/virnetdevtap.c | 1 - src/util/virstring.c | 53 +++ tests/testutils.c | 1 - tests/vboxsnapshotxmltest.c | 20 -- 13 files changed, 80 insertions(+), 155 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/12] util: use GRegex for virLogRegex
Signed-off-by: Ján Tomko
---
src/util/virlog.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index b3460d85fe..47e77e63b7 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -28,7 +28,6 @@
#include
#include
#include
-#include
#include
#if HAVE_SYSLOG_H
# include
@@ -61,7 +60,7 @@
VIR_LOG_INIT("util.log");
-static regex_t *virLogRegex;
+static GRegex *virLogRegex;
static char virLogHostname[HOST_NAME_MAX+1];
@@ -268,10 +267,7 @@ virLogOnceInit(void)
virLogLock();
virLogDefaultPriority = VIR_LOG_DEFAULT;
-if (VIR_ALLOC_QUIET(virLogRegex) >= 0) {
-if (regcomp(virLogRegex, VIR_LOG_REGEX, REG_EXTENDED) != 0)
-VIR_FREE(virLogRegex);
-}
+virLogRegex = g_regex_new(VIR_LOG_REGEX, G_REGEX_OPTIMIZE, 0, NULL);
/* We get and remember the hostname early, because at later time
* it might not be possible to load NSS modules via getaddrinfo()
@@ -1262,12 +1258,11 @@ virLogSetFromEnv(void)
*/
bool virLogProbablyLogMessage(const char *str)
{
-bool ret = false;
if (!virLogRegex)
return false;
-if (regexec(virLogRegex, str, 0, NULL, 0) == 0)
-ret = true;
-return ret;
+if (g_regex_match(virLogRegex, str, 0, NULL))
+return true;
+return false;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/12] util: use GRegex in virStringMatch
Signed-off-by: Ján Tomko
---
src/util/virstring.c | 22 --
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 3da793c87a..6add2278d8 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -19,7 +19,6 @@
#include
#include
-#include
#include
#include "c-ctype.h"
@@ -1089,24 +1088,19 @@ bool
virStringMatch(const char *str,
const char *regexp)
{
-regex_t re;
-int rv;
+g_autoptr(GRegex) regex = NULL;
+g_autoptr(GError) err = NULL;
VIR_DEBUG("match '%s' for '%s'", str, regexp);
-if ((rv = regcomp(, regexp, REG_EXTENDED | REG_NOSUB)) != 0) {
-char error[100];
-regerror(rv, , error, sizeof(error));
-VIR_WARN("error while compiling regular expression '%s': %s",
- regexp, error);
-return false;
+regex = g_regex_new(regexp, 0, 0, );
+if (!regex) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to compile regex %s"), err->message);
+return -1;
}
-rv = regexec(, str, 0, NULL, 0);
-
-regfree();
-
-return rv == 0;
+return g_regex_match(regex, str, 0, NULL);
}
/**
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/12] libxl: use GRegex in xenParseSxprVifRate
Use GRegex from GLib instead of regcomp.
Signed-off-by: Ján Tomko
---
src/libxl/xen_common.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index c06ab6e995..15a2db8add 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -24,8 +24,6 @@
#include
-#include
-
#include "internal.h"
#include "virerror.h"
#include "virconf.h"
@@ -1060,10 +1058,10 @@ static const char *vif_bytes_per_sec_re =
"^[0-9]+[GMK]?[Bb]/s$";
static int
xenParseSxprVifRate(const char *rate, unsigned long long *kbytes_per_sec)
{
+g_autoptr(GRegex) regex = NULL;
+g_autoptr(GError) err = NULL;
g_autofree char *trate = NULL;
char *p;
-regex_t rec;
-int err;
char *suffix;
unsigned long long tmp;
int ret = -1;
@@ -1074,17 +1072,14 @@ xenParseSxprVifRate(const char *rate, unsigned long
long *kbytes_per_sec)
if (p != NULL)
*p = 0;
-err = regcomp(, vif_bytes_per_sec_re, REG_EXTENDED|REG_NOSUB);
-if (err != 0) {
-char error[100];
-regerror(err, , error, sizeof(error));
+regex = g_regex_new(vif_bytes_per_sec_re, 0, 0, );
+if (!regex) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to compile regular expression '%s': %s"),
- vif_bytes_per_sec_re, error);
-goto cleanup;
+ _("Failed to compile regex %s"), err->message);
+return -1;
}
-if (regexec(, trate, 0, NULL, 0)) {
+if (!g_regex_match(regex, trate, 0, NULL)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid rate '%s' specified"), rate);
goto cleanup;
@@ -1108,7 +1103,6 @@ xenParseSxprVifRate(const char *rate, unsigned long long
*kbytes_per_sec)
ret = 0;
cleanup:
-regfree();
return ret;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/12] libxl: use GRegex in libxlGetAutoballoonConf
Replace the use of regcomp with GRegex.
Signed-off-by: Ján Tomko
---
src/libxl/libxl_conf.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 920f228d6a..be7d85b264 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -22,7 +22,6 @@
#include
-#include
#include
#include
#include
@@ -1664,7 +1663,8 @@ static int
libxlGetAutoballoonConf(libxlDriverConfigPtr cfg,
virConfPtr conf)
{
-regex_t regex;
+g_autoptr(GRegex) regex = NULL;
+g_autoptr(GError) err = NULL;
int res;
res = virConfGetValueBool(conf, "autoballoon", >autoballoon);
@@ -1673,21 +1673,15 @@ libxlGetAutoballoonConf(libxlDriverConfigPtr cfg,
else if (res == 1)
return 0;
-if ((res = regcomp(,
- "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
- REG_NOSUB | REG_EXTENDED)) != 0) {
-char error[100];
-regerror(res, , error, sizeof(error));
+regex = g_regex_new("(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($|
)",
+0, 0, );
+if (!regex) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to compile regex %s"),
- error);
-
+ _("Failed to compile regex %s"), err->message);
return -1;
}
-res = regexec(, cfg->verInfo->commandline, 0, NULL, 0);
-regfree();
-cfg->autoballoon = res == REG_NOMATCH;
+cfg->autoballoon = !g_regex_match(regex, cfg->verInfo->commandline, 0,
NULL);
return 0;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/12] remove unused regex.h includes
The code using regexes got moved, but the include stayed. Signed-off-by: Ján Tomko --- src/libxl/libxl_driver.c | 1 - src/storage/storage_util.c | 1 - src/util/viriscsi.c| 2 -- src/util/virnetdevtap.c| 1 - tests/testutils.c | 1 - 5 files changed, 6 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 277d99b2b0..ec6a010030 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -27,7 +27,6 @@ #include #include #include -#include #include "internal.h" #include "virlog.h" diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index f91c2c64ee..9ba9bb2a57 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -18,7 +18,6 @@ #include -#include #include #include #include diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 9f4c8f4e03..da27894b0b 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -22,8 +22,6 @@ #include -#include - #include "viriscsi.h" #include "viralloc.h" diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index aea4c91a6a..445aaa2727 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -32,7 +32,6 @@ #include "datatypes.h" #include -#include #include #include #include diff --git a/tests/testutils.c b/tests/testutils.c index a3bedd0fef..3cd3391dfa 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include "testutils.h" -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/12] util: use GRegex in virStringSearch
Signed-off-by: Ján Tomko
---
src/util/virstring.c | 31 ++-
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 283cf8c8d8..3da793c87a 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -1017,29 +1017,27 @@ virStringSearch(const char *str,
size_t max_matches,
char ***matches)
{
-regex_t re;
-regmatch_t rem;
+g_autoptr(GRegex) regex = NULL;
+g_autoptr(GError) err = NULL;
+g_autoptr(GMatchInfo) info = NULL;
size_t nmatches = 0;
ssize_t ret = -1;
-int rv = -1;
*matches = NULL;
VIR_DEBUG("search '%s' for '%s'", str, regexp);
-if ((rv = regcomp(, regexp, REG_EXTENDED)) != 0) {
-char error[100];
-regerror(rv, , error, sizeof(error));
+regex = g_regex_new(regexp, 0, 0, );
+if (!regex) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Error while compiling regular expression '%s': %s"),
- regexp, error);
+ _("Failed to compile regex %s"), err->message);
return -1;
}
-if (re.re_nsub != 1) {
+if (g_regex_get_capture_count(regex) != 1) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Regular expression '%s' must have exactly 1 match
group, not %zu"),
- regexp, re.re_nsub);
+ _("Regular expression '%s' must have exactly 1 match
group, not %d"),
+ regexp, g_regex_get_capture_count(regex));
goto cleanup;
}
@@ -1051,28 +1049,27 @@ virStringSearch(const char *str,
while ((nmatches - 1) < max_matches) {
char *match;
+int endpos;
-if (regexec(, str, 1, , 0) != 0)
+if (!g_regex_match(regex, str, 0, ))
break;
if (VIR_EXPAND_N(*matches, nmatches, 1) < 0)
goto cleanup;
-if (VIR_STRNDUP(match, str + rem.rm_so,
-rem.rm_eo - rem.rm_so) < 0)
-goto cleanup;
+match = g_match_info_fetch(info, 1);
VIR_DEBUG("Got '%s'", match);
(*matches)[nmatches-2] = match;
-str = str + rem.rm_eo;
+g_match_info_fetch_pos(info, 1, NULL, );
+str += endpos;
}
ret = nmatches - 1; /* don't count the trailing null */
cleanup:
-regfree();
if (ret < 0) {
virStringListFree(*matches);
*matches = NULL;
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] docs: some refactoring and new docs about RPM deployment
On Wed, Nov 13, 2019 at 02:53:21PM +0100, Kashyap Chamarthy wrote: > On Mon, Nov 11, 2019 at 02:39:35PM +, Daniel P. Berrangé wrote: > > [...] > > > > NB writing content is RST is independent of eliminating XSLT as the > > templating system. Elimination oif XSLT depends what templating > > system we use, either Sphinx or Pelican - I've not delved into it > > to figure out which is better, > > FWIW, Sphinx seems to be the "preferred" choice for Linux[*], QEMU. > (OpenStack upstream also uses it heavily.) Yep, but there's a difference for libvirt's needs. IIUC those projects are all building the docs into a manual. Libvirt is building its docs into a website with a series of pages. I've not investigated in a lot of detail, but my first impressions are that Pelican is better suited for building websites, while Sphinx is better suited for building manuals. Both use RST as their input though, so the decision of which to use mostly impacts the way we later replace the XSLT with a new page template, so I've not spent much time on it yet. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] qemu: block: Fix two bugs related to usage of blockdev (blockdev-add saga)
Peter Krempa (2): qemu: blockjob: Transfer 'readonly' state of images after active layer block commit qemu: snapshot: Fix inactive external snapshots when backing chain is present src/qemu/qemu_blockjob.c | 2 ++ src/qemu/qemu_driver.c | 25 +++-- 2 files changed, 21 insertions(+), 6 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] qemu: blockjob: Transfer 'readonly' state of images after active layer block commit
When commiting a different image becomes the disk source. Since we store the readonly flag per-image we must update it to the same state the original image had. Signed-off-by: Peter Krempa --- src/qemu/qemu_blockjob.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 2dddb1e408..5c294f8024 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -1106,6 +1106,7 @@ qemuBlockJobProcessEventCompletedActiveCommit(virQEMUDriverPtr driver, cfgbase = cfgbaseparent->backingStore; cfgbaseparent->backingStore = NULL; cfgdisk->src = cfgbase; +cfgdisk->src->readonly = cfgtop->readonly; virObjectUnref(cfgtop); } @@ -1115,6 +1116,7 @@ qemuBlockJobProcessEventCompletedActiveCommit(virQEMUDriverPtr driver, baseparent->backingStore = NULL; job->disk->src = job->data.commit.base; +job->disk->src->readonly = job->data.commit.top->readonly; qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, job->data.commit.top); virObjectUnref(job->data.commit.top); -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] qemu: snapshot: Fix inactive external snapshots when backing chain is present
The inactive external snapshot code replaced the file name in the
virStorageSource but did not touch the backing files. This meant that
after an inactive snapshot the backing chain recorded in the inactive
XML (which is used with -blockdev) would be incorrect.
Fix it by adding a new layer if there is an existing chain and replacing
the virStorageSource struct fully when there is no chain.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 25 +++--
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c969a3d463..c1b404adfa 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14613,19 +14613,32 @@
qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver,
/* update disk definitions */
for (i = 0; i < snapdef->ndisks; i++) {
+g_autoptr(virStorageSource) newsrc = NULL;
+
snapdisk = &(snapdef->disks[i]);
defdisk = vm->def->disks[snapdisk->idx];
-if (snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
-VIR_FREE(defdisk->src->path);
-defdisk->src->path = g_strdup(snapdisk->src->path);
-defdisk->src->format = snapdisk->src->format;
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
-if (virDomainSaveConfig(cfg->configDir, driver->caps, vm->def) < 0)
-goto cleanup;
+if (!(newsrc = virStorageSourceCopy(snapdisk->src, false)))
+goto cleanup;
+
+if (virStorageSourceInitChainElement(newsrc, defdisk->src, false) < 0)
+goto cleanup;
+
+if (virStorageSourceHasBacking(defdisk->src)) {
+newsrc->backingStore = g_steal_pointer(>src);
+} else {
+virObjectUnref(defdisk->src);
}
+
+defdisk->src = g_steal_pointer();
}
+if (virDomainSaveConfig(cfg->configDir, driver->caps, vm->def) < 0)
+goto cleanup;
+
ret = 0;
cleanup:
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 9/9] docs: hacking: document removal of VIR_STR(N)DUP
On 11/12/19 2:02 PM, Ján Tomko wrote: Signed-off-by: Ján Tomko --- docs/hacking.html.in | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Daniel Henrique Barboza -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] bootstrap.conf: remove usage of snprintf gnulib module
Signed-off-by: Pavel Hrdina --- bootstrap.conf | 1 - 1 file changed, 1 deletion(-) diff --git a/bootstrap.conf b/bootstrap.conf index 4c784487e2..747040a1a3 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -74,7 +74,6 @@ setenv setsockopt sigaction sigpipe -snprintf socket stat-time strchrnul -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] replace snprintf by g_snprintf (kill-a-gnulib-module-a-day initiative)
Pavel Hrdina (2): replace use of gnulib snprintf by g_snprintf bootstrap.conf: remove usage of snprintf gnulib module bootstrap.conf| 1 - src/hyperv/hyperv_driver.c| 4 +- src/libxl/libxl_conf.c| 22 +-- src/libxl/libxl_migration.c | 4 +- src/lxc/lxc_process.c | 6 +-- src/nwfilter/nwfilter_ebiptables_driver.c | 36 +- src/nwfilter/nwfilter_learnipaddr.c | 2 +- src/openvz/openvz_driver.c| 6 +-- src/qemu/qemu_agent.c | 46 +++ src/qemu/qemu_driver.c| 8 ++-- src/security/security_dac.c | 8 ++-- src/security/virt-aa-helper.c | 2 +- src/storage/storage_util.c| 2 +- src/util/virhostcpu.c | 2 +- src/util/viriptables.c| 6 +-- src/util/virmacaddr.c | 8 ++-- src/util/virnetdevbridge.c| 4 +- src/util/virnetdevmacvlan.c | 12 +++--- src/util/virpci.c | 4 +- src/util/virpidfile.c | 4 +- src/util/virtime.c| 10 ++--- src/util/virusb.c | 8 ++-- src/util/viruuid.c| 12 +++--- src/vbox/vbox_common.c| 24 ++-- src/vbox/vbox_tmpl.c | 2 +- src/vmx/vmx.c | 18 - src/vz/vz_driver.c| 22 +-- src/vz/vz_sdk.c | 8 ++-- tests/libxlmock.c | 2 +- tests/qemuagenttest.c | 12 +++--- tests/testutils.c | 2 +- tests/virnetsockettest.c | 2 +- tests/virpcimock.c| 28 +++--- tests/virsystemdtest.c| 4 +- tools/virsh-domain-monitor.c | 2 +- tools/virsh-domain.c | 2 +- tools/virt-host-validate-common.c | 2 +- tools/virt-login-shell.c | 4 +- tools/vsh-table.c | 6 +-- tools/vsh.c | 22 +-- tools/wireshark/src/packet-libvirt.c | 2 +- 41 files changed, 190 insertions(+), 191 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] replace use of gnulib snprintf by g_snprintf
Glib implementation follows the ISO C99 standard so it's safe to replace
the gnulib implementation.
Signed-off-by: Pavel Hrdina
---
src/hyperv/hyperv_driver.c| 4 +-
src/libxl/libxl_conf.c| 22 +--
src/libxl/libxl_migration.c | 4 +-
src/lxc/lxc_process.c | 6 +--
src/nwfilter/nwfilter_ebiptables_driver.c | 36 +-
src/nwfilter/nwfilter_learnipaddr.c | 2 +-
src/openvz/openvz_driver.c| 6 +--
src/qemu/qemu_agent.c | 46 +++
src/qemu/qemu_driver.c| 8 ++--
src/security/security_dac.c | 8 ++--
src/security/virt-aa-helper.c | 2 +-
src/storage/storage_util.c| 2 +-
src/util/virhostcpu.c | 2 +-
src/util/viriptables.c| 6 +--
src/util/virmacaddr.c | 8 ++--
src/util/virnetdevbridge.c| 4 +-
src/util/virnetdevmacvlan.c | 12 +++---
src/util/virpci.c | 4 +-
src/util/virpidfile.c | 4 +-
src/util/virtime.c| 10 ++---
src/util/virusb.c | 8 ++--
src/util/viruuid.c| 12 +++---
src/vbox/vbox_common.c| 24 ++--
src/vbox/vbox_tmpl.c | 2 +-
src/vmx/vmx.c | 18 -
src/vz/vz_driver.c| 22 +--
src/vz/vz_sdk.c | 8 ++--
tests/libxlmock.c | 2 +-
tests/qemuagenttest.c | 12 +++---
tests/testutils.c | 2 +-
tests/virnetsockettest.c | 2 +-
tests/virpcimock.c| 28 +++---
tests/virsystemdtest.c| 4 +-
tools/virsh-domain-monitor.c | 2 +-
tools/virsh-domain.c | 2 +-
tools/virt-host-validate-common.c | 2 +-
tools/virt-login-shell.c | 4 +-
tools/vsh-table.c | 6 +--
tools/vsh.c | 22 +--
tools/wireshark/src/packet-libvirt.c | 2 +-
40 files changed, 190 insertions(+), 190 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index ac10f86b82..3a2f6602bc 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -1381,7 +1381,7 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int
codeset,
/* press the keys */
for (i = 0; i < nkeycodes; i++) {
-snprintf(keycodeStr, sizeof(keycodeStr), "%d", translatedKeycodes[i]);
+g_snprintf(keycodeStr, sizeof(keycodeStr), "%d",
translatedKeycodes[i]);
params = hypervCreateInvokeParamsList(priv, "PressKey", selector,
Msvm_Keyboard_WmiInfo);
@@ -1409,7 +1409,7 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int
codeset,
/* release the keys */
for (i = 0; i < nkeycodes; i++) {
-snprintf(keycodeStr, sizeof(keycodeStr), "%d", translatedKeycodes[i]);
+g_snprintf(keycodeStr, sizeof(keycodeStr), "%d",
translatedKeycodes[i]);
params = hypervCreateInvokeParamsList(priv, "ReleaseKey", selector,
Msvm_Keyboard_WmiInfo);
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 920f228d6a..71a503c0f7 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -419,12 +419,12 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
continue;
}
-snprintf(xlCPU,
-sizeof(xlCPU),
-"%s=0",
-xenTranslateCPUFeature(
-def->cpu->features[i].name,
-false));
+g_snprintf(xlCPU,
+ sizeof(xlCPU),
+ "%s=0",
+ xenTranslateCPUFeature(
+ def->cpu->features[i].name,
+ false));
if (libxl_cpuid_parse_config(_info->cpuid,
xlCPU)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported cpu feature '%s'"),
@@ -441,11 +441,11 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
continue;
}
-snprintf(xlCPU,
-sizeof(xlCPU),
-"%s=1",
-xenTranslateCPUFeature(
-
Re: [libvirt] [PATCH 8/9] util: remove VIR_STRDUP and VIR_STRNDUP
On 11/12/19 2:02 PM, Ján Tomko wrote: Signed-off-by: Ján Tomko --- src/libvirt_private.syms | 2 -- src/util/virstring.c | 49 src/util/virstring.h | 69 3 files changed, 120 deletions(-) (assuming the VIR_STRNDUP introduced by Jirka's commit 0861080f0e23921 is taken care of) Reviewed-by: Daniel Henrique Barboza -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] docs: some refactoring and new docs about RPM deployment
On Mon, Nov 11, 2019 at 02:39:35PM +, Daniel P. Berrangé wrote: [...] > NB writing content is RST is independent of eliminating XSLT as the > templating system. Elimination oif XSLT depends what templating > system we use, either Sphinx or Pelican - I've not delved into it > to figure out which is better, FWIW, Sphinx seems to be the "preferred" choice for Linux[*], QEMU. (OpenStack upstream also uses it heavily.) [...] [*] https://www.kernel.org/doc/html/v4.11/doc-guide/sphinx.html -- /kashyap -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8] Remove use of 'areadlink' gnulib module (kill-a-gnulib-module-a-day initiative)
On Wed, Nov 13, 2019 at 14:07:01 +0100, Peter Krempa wrote: > This removes use of 'areadlink' by using 'g_file_read_link' and fixes a > few insane uses. > > Note that I'll post the actual module deletion from bootstrap.conf > separately as touching that file results in lengthy rebuilds. The commits actually deleting the modules from bootstrap.conf will be agreggated here: https://gitlab.com/pipo.sk/libvirt/commits/gnulib -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/8] util: pidfile: Sanitize return values of virPidFileReadPathIfAlive
The callers don't actually use the returned errno for reporting errors.
Additionally virFileResolveAllLinks returns -1 rather than -errno on
error thus you'd get a spurious EPERM even on other errors.
Don't try to return errno in this case.
Signed-off-by: Peter Krempa
---
src/util/virpidfile.c | 50 +--
1 file changed, 24 insertions(+), 26 deletions(-)
diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
index b1b8e54993..4a800b6528 100644
--- a/src/util/virpidfile.c
+++ b/src/util/virpidfile.c
@@ -182,7 +182,7 @@ int virPidFileRead(const char *dir,
* If @binpath is NULL the check for the executable path
* is skipped.
*
- * Returns -errno upon error, or zero on successful
+ * Returns -1 upon error, or zero on successful
* reading of the pidfile. If the PID was not still
* alive, zero will be returned, but @pid will be
* set to -1.
@@ -191,8 +191,8 @@ int virPidFileReadPathIfAlive(const char *path,
pid_t *pid,
const char *binPath)
{
-int ret;
-bool isLink;
+int rc;
+bool isLink = false;
size_t procLinkLen;
const char deletedText[] = " (deleted)";
size_t deletedTextLen = strlen(deletedText);
@@ -205,16 +205,15 @@ int virPidFileReadPathIfAlive(const char *path,
/* only set this at the very end on success */
*pid = -1;
-if ((ret = virPidFileReadPath(path, )) < 0)
-return ret;
+if (virPidFileReadPath(path, ) < 0)
+return -1;
#ifndef WIN32
/* Check that it's still alive. Safe to skip this sanity check on
* mingw, which lacks kill(). */
if (kill(retPid, 0) < 0) {
-ret = 0;
-retPid = -1;
-goto cleanup;
+*pid = -1;
+return 0;
}
#endif
@@ -222,23 +221,24 @@ int virPidFileReadPathIfAlive(const char *path,
/* we only knew the pid, and that pid is alive, so we can
* return it.
*/
-ret = 0;
-goto cleanup;
+*pid = retPid;
+return 0;
}
procPath = g_strdup_printf("/proc/%lld/exe", (long long)retPid);
-if ((ret = virFileIsLink(procPath)) < 0)
-return ret;
+if ((rc = virFileIsLink(procPath)) < 0)
+return -1;
-isLink = ret;
+if (rc == 1)
+isLink = true;
if (isLink && virFileLinkPointsTo(procPath, binPath)) {
/* the link in /proc/$pid/exe is a symlink to a file
* that has the same inode as the file at binpath.
*/
-ret = 0;
-goto cleanup;
+*pid = retPid;
+return 0;
}
/* Even if virFileLinkPointsTo returns a mismatch, it could be
@@ -248,24 +248,22 @@ int virPidFileReadPathIfAlive(const char *path,
* part, and see if it has the same canonicalized name as binpath.
*/
if (!(procLink = areadlink(procPath)))
-return -errno;
+return -1;
procLinkLen = strlen(procLink);
if (procLinkLen > deletedTextLen)
procLink[procLinkLen - deletedTextLen] = 0;
-if ((ret = virFileResolveAllLinks(binPath, )) < 0)
-return ret;
-if ((ret = virFileResolveAllLinks(procLink, )) < 0)
-return ret;
+if (virFileResolveAllLinks(binPath, ) < 0)
+return -1;
+if (virFileResolveAllLinks(procLink, ) < 0)
+return -1;
-ret = STREQ(resolvedBinPath, resolvedProcLink) ? 0 : -1;
+if (STRNEQ(resolvedBinPath, resolvedProcLink))
+return -1;
- cleanup:
-/* return the originally set pid of -1 unless we proclaim success */
-if (ret == 0)
-*pid = retPid;
-return ret;
+*pid = retPid;
+return 0;
}
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/8] util: pidfile: Sanitize return values of virPidFileReadIfAlive
Return -1 on failure rather than -errno since none of the callers actually cares about the return value. This specifically fixes returns of -ENOMEM in cases of bad usage, which would report wrong error anyways. Signed-off-by: Peter Krempa --- src/util/virpidfile.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c index 3e78735c9c..b1b8e54993 100644 --- a/src/util/virpidfile.c +++ b/src/util/virpidfile.c @@ -282,7 +282,7 @@ int virPidFileReadPathIfAlive(const char *path, * and its executable path resolves to @binpath. This adds * protection against recycling of previously reaped pids. * - * Returns -errno upon error, or zero on successful + * Returns -1 upon error, or zero on successful * reading of the pidfile. If the PID was not still * alive, zero will be returned, but @pid will be * set to -1. @@ -295,12 +295,15 @@ int virPidFileReadIfAlive(const char *dir, g_autofree char *pidfile = NULL; if (name == NULL || dir == NULL) -return -EINVAL; +return -1; if (!(pidfile = virPidFileBuildPath(dir, name))) -return -ENOMEM; +return -1; + +if (virPidFileReadPathIfAlive(pidfile, pid, binpath) < 0) +return -1; -return virPidFileReadPathIfAlive(pidfile, pid, binpath); +return 0; } -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/8] util: pidfile: Replace 'areadlink' by 'g_file_read_link'
Use the glib function rather than gnulib. Signed-off-by: Peter Krempa --- src/util/virpidfile.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c index 4a800b6528..5d1a820cd1 100644 --- a/src/util/virpidfile.c +++ b/src/util/virpidfile.c @@ -35,7 +35,6 @@ #include "virlog.h" #include "virerror.h" #include "c-ctype.h" -#include "areadlink.h" #include "virstring.h" #include "virprocess.h" @@ -247,7 +246,7 @@ int virPidFileReadPathIfAlive(const char *path, * "$procpath (deleted)". Read that link, remove the " (deleted)" * part, and see if it has the same canonicalized name as binpath. */ -if (!(procLink = areadlink(procPath))) +if (!(procLink = g_file_read_link(procPath, NULL))) return -1; procLinkLen = strlen(procLink); -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/8] qemu: tpm: Sanitize error values in qemuTPMEmulatorGetPid
The callers don't care about the actual return value, so return -1
rather than errno.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_tpm.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 40136c4410..2f92542470 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -266,7 +266,7 @@ qemuTPMEmulatorCreatePidFilename(const char *swtpmStateDir,
* @shortName: short name of the domain
* @pid: pointer to pid
*
- * Return -errno upon error, or zero on successful reading of the pidfile.
+ * Return -1 upon error, or zero on successful reading of the pidfile.
* If the PID was not still alive, zero will be returned, and @pid will be
* set to -1;
*/
@@ -275,16 +275,16 @@ qemuTPMEmulatorGetPid(const char *swtpmStateDir,
const char *shortName,
pid_t *pid)
{
-int ret;
g_autofree char *swtpm = virTPMGetSwtpm();
g_autofree char *pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir,
shortName);
if (!pidfile)
-return -ENOMEM;
+return -1;
-ret = virPidFileReadPathIfAlive(pidfile, pid, swtpm);
+if (virPidFileReadPathIfAlive(pidfile, pid, swtpm) < 0)
+return -1;
-return ret;
+return 0;
}
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/8] qemu: gpu: Sanitize error values in qemuVhostUserGPUGetPid
The caller doesn't care about the actual return value, so return -1
rather than errno.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_vhost_user_gpu.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c
index 37e424eac3..51244f9b35 100644
--- a/src/qemu/qemu_vhost_user_gpu.c
+++ b/src/qemu/qemu_vhost_user_gpu.c
@@ -61,7 +61,7 @@ qemuVhostUserGPUCreatePidFilename(const char *stateDir,
* @alias: video device alias
* @pid: pointer to pid
*
- * Return -errno upon error, or zero on successful reading of the pidfile.
+ * Return -1 upon error, or zero on successful reading of the pidfile.
* If the PID was not still alive, zero will be returned, and @pid will be
* set to -1;
*/
@@ -74,11 +74,13 @@ qemuVhostUserGPUGetPid(const char *binPath,
{
g_autofree char *pidfile = NULL;
-pidfile = qemuVhostUserGPUCreatePidFilename(stateDir, shortName, alias);
-if (!pidfile)
-return -ENOMEM;
+if (!(pidfile = qemuVhostUserGPUCreatePidFilename(stateDir, shortName,
alias)))
+return -1;
+
+if (virPidFileReadPathIfAlive(pidfile, pid, binPath) < 0)
+return -1;
-return virPidFileReadPathIfAlive(pidfile, pid, binPath);
+return 0;
}
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/8] Remove use of 'areadlink' gnulib module (kill-a-gnulib-module-a-day initiative)
This removes use of 'areadlink' by using 'g_file_read_link' and fixes a few insane uses. Note that I'll post the actual module deletion from bootstrap.conf separately as touching that file results in lengthy rebuilds. Peter Krempa (8): qemu: domain: Use g_file_read_link instead of virFileReadLink util: file: Remove virFileReadLink qemu: tpm: Use g_autofree in qemuTPMEmulatorGetPid qemu: tpm: Sanitize error values in qemuTPMEmulatorGetPid qemu: gpu: Sanitize error values in qemuVhostUserGPUGetPid util: pidfile: Sanitize return values of virPidFileReadIfAlive util: pidfile: Sanitize return values of virPidFileReadPathIfAlive util: pidfile: Replace 'areadlink' by 'g_file_read_link' src/libvirt_private.syms | 1 - src/qemu/qemu_domain.c | 18 +- src/qemu/qemu_tpm.c| 16 - src/qemu/qemu_vhost_user_gpu.c | 12 --- src/util/virfile.c | 13 --- src/util/virfile.h | 3 -- src/util/virpidfile.c | 64 +- 7 files changed, 56 insertions(+), 71 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/8] qemu: domain: Use g_file_read_link instead of virFileReadLink
In an effort to remove as much gnulib usage as possible let's
reimplement virFileReadLink. Since it's used in two places only I opted
to open-code it.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 842b70dc26..40a617845a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -13170,12 +13170,13 @@ qemuDomainCreateDeviceRecursive(const char *device,
}
if (isLink) {
+g_autoptr(GError) gerr = NULL;
+
/* We are dealing with a symlink. Create a dangling symlink and descend
* down one level which hopefully creates the symlink's target. */
-if (virFileReadLink(device, ) < 0) {
-virReportSystemError(errno,
- _("unable to resolve symlink %s"),
- device);
+if (!(target = g_file_read_link(device, ))) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to resolve symlink %s: %s"), device,
gerr->message);
goto cleanup;
}
@@ -14164,10 +14165,11 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr
driver,
data.target = target;
} else if (isLink) {
-if (virFileReadLink(file, ) < 0) {
-virReportSystemError(errno,
- _("unable to resolve symlink %s"),
- file);
+g_autoptr(GError) gerr = NULL;
+
+if (!(target = g_file_read_link(file, ))) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to resolve symlink %s: %s"), file,
gerr->message);
return ret;
}
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/8] util: file: Remove virFileReadLink
The function is unused so we can remove it.
Signed-off-by: Peter Krempa
---
src/libvirt_private.syms | 1 -
src/util/virfile.c | 13 -
src/util/virfile.h | 3 ---
3 files changed, 17 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4d0d03c580..c706553e37 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1978,7 +1978,6 @@ virFileReadBufQuiet;
virFileReadHeaderFD;
virFileReadHeaderQuiet;
virFileReadLimFD;
-virFileReadLink;
virFileReadValueBitmap;
virFileReadValueInt;
virFileReadValueScaledInt;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index ced0ea70b7..b1aeaa5851 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -81,7 +81,6 @@
#include "virutil.h"
#include "c-ctype.h"
-#include "areadlink.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -1625,18 +1624,6 @@ virFileIsLink(const char *linkpath)
return S_ISLNK(st.st_mode) != 0;
}
-/*
- * Read where symlink is pointing to.
- *
- * Returns 0 on success (@linkpath is a successfully read link),
- *-1 with errno set upon error.
- */
-int
-virFileReadLink(const char *linkpath, char **resultpath)
-{
-return (*resultpath = areadlink(linkpath)) ? 0 : -1;
-}
-
/*
* Finds a requested executable file in the PATH env. e.g.:
* "qemu-img" will return "/usr/bin/qemu-img"
diff --git a/src/util/virfile.h b/src/util/virfile.h
index a570529330..9a8709b52c 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -176,9 +176,6 @@ int virFileResolveAllLinks(const char *linkpath,
int virFileIsLink(const char *linkpath)
ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT;
-int virFileReadLink(const char *linkpath, char **resultpath)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT;
-
char *virFindFileInPath(const char *file);
char *virFileFindResource(const char *filename,
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/8] qemu: tpm: Use g_autofree in qemuTPMEmulatorGetPid
Signed-off-by: Peter Krempa
---
src/qemu/qemu_tpm.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 62b9f803bd..40136c4410 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -277,15 +277,13 @@ qemuTPMEmulatorGetPid(const char *swtpmStateDir,
{
int ret;
g_autofree char *swtpm = virTPMGetSwtpm();
-char *pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir,
- shortName);
+g_autofree char *pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir,
+shortName);
if (!pidfile)
return -ENOMEM;
ret = virPidFileReadPathIfAlive(pidfile, pid, swtpm);
-VIR_FREE(pidfile);
-
return ret;
}
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/9] tests: delete tests for VIR_STR(N)DUP
On 11/12/19 2:02 PM, Ján Tomko wrote: Signed-off-by: Ján Tomko --- tests/virstringtest.c | 136 -- 1 file changed, 136 deletions(-) Reviewed-by: Daniel Henrique Barboza -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/9] Remove the rest of VIR_STRNDUP
On 11/12/19 2:02 PM, Ján Tomko wrote: Replace all the uses passing a single parameter as the length. Signed-off-by: Ján Tomko --- src/conf/nwfilter_conf.c | 4 ++-- src/conf/nwfilter_params.c | 6 ++ src/interface/interface_backend_udev.c | 8 ++-- src/libxl/xen_common.c | 3 +-- src/libxl/xen_xl.c | 6 ++ src/libxl/xen_xm.c | 6 ++ src/qemu/qemu_monitor_json.c | 3 +-- src/rpc/virnetlibsshsession.c | 7 ++- src/storage/storage_backend_logical.c | 7 ++- src/util/viriscsi.c| 3 +-- src/util/virjson.c | 11 +++ src/util/virkeyfile.c | 3 +-- src/util/virsocketaddr.c | 5 + src/util/virstoragefile.c | 3 +-- src/util/virstring.c | 3 +-- 15 files changed, 24 insertions(+), 54 deletions(-) [...] diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 9f4c8f4e03..84fa542b7c 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -162,8 +162,7 @@ virStorageBackendIQNFound(const char *initiatoriqn, if (!(next = strchr(current, ' '))) goto error; -if (VIR_STRNDUP(iface, current, (next - current)) < 0) -goto cleanup; +iface = g_strndup(current, (next - current)); current = next + 1; We can live without the parenthesis around 'next - current'. Also, I think this change belongs to the previous patch that handled subtraction in the length argument of VIR_STRNDUP(). Thanks, DHB -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: mention lifted vCPUs restriction for esx
It was lifted with c92b6023e8eb670e01571e299a85e9da9bd4844c. Signed-off-by: Pino Toscano --- docs/drvesx.html.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in index 901d65958a..ac7bc645d1 100644 --- a/docs/drvesx.html.in +++ b/docs/drvesx.html.in @@ -337,7 +337,9 @@ error: invalid argument in libvirt was built without the 'esx' driver Memory size has to be a multiple of 4096 -Number of virtual CPU has to be 1 or a multiple of 2 +Number of virtual CPU has to be 1 or a multiple of 2. +Since 4.10.0 any number of vCPUs is +supported. Valid MAC address prefixes are 00:0c:29 and -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH Rust 1/4] libvirt-rust: stream: add more functions in stream
On Tue, Nov 12, 2019 at 02:44:42PM -0700, Zixing Liu wrote:
> * added new functions: virStreamNew virStreamEventUpdateCallback
> virStreamEventRemoveCallback
> * added new constants: VIR_STREAM_NONBLOCK
> * added new types/aliases: StreamFlags
> * changed the previous `new` associate function to `from_ptr` to avoid
> naming conflicts
> * added basic tests to test the creation of Stream struct (can not test
> the correctness of virStreamEventRemoveCallback and
> virStreamEventUpdateCallback due to "unsupported by the connection
> driver")
>
> Signed-off-by: Zixing Liu
> ---
> src/stream.rs | 43 +--
> tests/stream.rs | 40
> 2 files changed, 81 insertions(+), 2 deletions(-)
> create mode 100644 tests/stream.rs
>
> diff --git a/src/stream.rs b/src/stream.rs
> index 2ca5d0b..de272ab 100644
> --- a/src/stream.rs
> +++ b/src/stream.rs
> @@ -18,8 +18,9 @@
>
> extern crate libc;
>
> +use connect::Connect;
> +use connect::sys::virConnectPtr;
> use std::convert::TryFrom;
> -use std::str;
>
> use error::Error;
>
> @@ -32,6 +33,9 @@ pub mod sys {
>
> #[link(name = "virt")]
> extern "C" {
> +fn virStreamNew(c: virConnectPtr,
> +flags: libc::c_uint)
> + -> sys::virStreamPtr;
> fn virStreamSend(c: sys::virStreamPtr,
> data: *const libc::c_char,
> nbytes: libc::size_t)
> @@ -43,6 +47,9 @@ extern "C" {
> fn virStreamFree(c: sys::virStreamPtr) -> libc::c_int;
> fn virStreamAbort(c: sys::virStreamPtr) -> libc::c_int;
> fn virStreamFinish(c: sys::virStreamPtr) -> libc::c_int;
> +fn virStreamEventUpdateCallback(c: sys::virStreamPtr,
> +events: libc::c_int) -> libc::c_int;
> +fn virStreamEventRemoveCallback(c: sys::virStreamPtr) -> libc::c_int;
> }
>
> pub type StreamEventType = self::libc::c_uint;
> @@ -51,6 +58,9 @@ pub const VIR_STREAM_EVENT_WRITABLE: StreamEventType = (1
> << 1);
> pub const VIR_STREAM_EVENT_ERROR: StreamEventType = (1 << 2);
> pub const VIR_STREAM_EVENT_HANGUP: StreamEventType = (1 << 3);
>
> +pub type StreamFlags = self::libc::c_uint;
> +pub const VIR_STREAM_NONBLOCK: StreamFlags = (1 << 0);
> +
> #[derive(Debug)]
> pub struct Stream {
> ptr: Option,
> @@ -69,7 +79,17 @@ impl Drop for Stream {
> }
>
> impl Stream {
> -pub fn new(ptr: sys::virStreamPtr) -> Stream {
> +pub fn new(conn: , flags: StreamFlags) -> Result {
> +unsafe {
We probably just want to use unsafe when calling virStreamNew, and we
should try the most as possible to explain why it is safe to use
"unsafe". Basically here, we are safe because we specifically
allocated conn at Rust level.
> +let ptr = virStreamNew(conn.as_ptr(), flags as libc::c_uint);
> +if ptr.is_null() {
> +return Err(Error::new());
> +}
> +return Ok(Stream::from_ptr(ptr));
> +}
> +}
> +
> +pub fn from_ptr(ptr: sys::virStreamPtr) -> Stream {
> Stream { ptr: Some(ptr) }
> }
Yes I think using 'from_ptr' would make more sense. We probably have
to update that everywhere also I don't think we want have that as
public.
Can you just remove the 'pub' flag of that function?
> @@ -126,4 +146,23 @@ impl Stream {
> };
> usize::try_from(ret).map_err(|_| Error::new())
> }
> +
> +pub fn event_update_callback(, events: StreamEventType) ->
> Result<(), Error> {
> +let ret = unsafe {
Same comment about usage of 'unsafe'.
> +virStreamEventUpdateCallback(self.as_ptr(), events as
> libc::c_int)
We defined StreamEventType as unsigned.
> +};
> +if ret == -1 {
> +return Err(Error::new());
> +}
> +return Ok(());
> +}
> +
> +pub fn event_remove_callback() -> Result<(), Error> {
> +unsafe {
Same about unsafe.
> +if virStreamEventRemoveCallback(self.as_ptr()) == -1 {
> +return Err(Error::new());
> +}
> +return Ok(());
> +}
> +}
> }
> diff --git a/tests/stream.rs b/tests/stream.rs
> new file mode 100644
> index 000..16531b3
> --- /dev/null
> +++ b/tests/stream.rs
> @@ -0,0 +1,40 @@
> +/*
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this
Re: [libvirt] [PATCH 5/9] Remove VIR_STRNDUP usage with subtraction
On 11/12/19 2:02 PM, Ján Tomko wrote: Remove all the uses that use subtraction in their length argument. Signed-off-by: Ján Tomko --- Reviewed-by: Daniel Henrique Barboza -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] esx: split datastorePoolType helper
Move the detection of the type of a vmfs pool out of
esxLookupVMFSStoragePoolType in an own helper. This way it can be used
also in other functions.
Signed-off-by: Pino Toscano
---
src/esx/esx_storage_backend_vmfs.c | 49 --
1 file changed, 33 insertions(+), 16 deletions(-)
diff --git a/src/esx/esx_storage_backend_vmfs.c
b/src/esx/esx_storage_backend_vmfs.c
index 78fe2b598d..b890825a40 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -54,26 +54,12 @@ verify(VIR_CRYPTO_HASH_SIZE_MD5 == VIR_UUID_BUFLEN);
static int
-esxLookupVMFSStoragePoolType(esxVI_Context *ctx, const char *poolName,
- int *poolType)
+datastorePoolType(esxVI_ObjectContent *datastore, int *poolType)
{
int result = -1;
-esxVI_String *propertyNameList = NULL;
-esxVI_ObjectContent *datastore = NULL;
esxVI_DynamicProperty *dynamicProperty = NULL;
esxVI_DatastoreInfo *datastoreInfo = NULL;
-if (esxVI_String_AppendValueToList(, "info") < 0 ||
-esxVI_LookupDatastoreByName(ctx, poolName, propertyNameList,
,
-esxVI_Occurrence_OptionalItem) < 0) {
-goto cleanup;
-}
-
-if (!datastore) {
-/* Not found, let the base storage driver handle error reporting */
-goto cleanup;
-}
-
for (dynamicProperty = datastore->propSet; dynamicProperty;
dynamicProperty = dynamicProperty->_next) {
if (STREQ(dynamicProperty->name, "info")) {
@@ -100,10 +86,41 @@ esxLookupVMFSStoragePoolType(esxVI_Context *ctx, const
char *poolName,
result = 0;
+ cleanup:
+esxVI_DatastoreInfo_Free();
+
+return result;
+}
+
+
+
+static int
+esxLookupVMFSStoragePoolType(esxVI_Context *ctx, const char *poolName,
+ int *poolType)
+{
+int result = -1;
+esxVI_String *propertyNameList = NULL;
+esxVI_ObjectContent *datastore = NULL;
+
+if (esxVI_String_AppendValueToList(, "info") < 0 ||
+esxVI_LookupDatastoreByName(ctx, poolName, propertyNameList,
,
+esxVI_Occurrence_OptionalItem) < 0) {
+goto cleanup;
+}
+
+if (!datastore) {
+/* Not found, let the base storage driver handle error reporting */
+goto cleanup;
+}
+
+if (datastorePoolType(datastore, poolType) < 0)
+goto cleanup;
+
+result = 0;
+
cleanup:
esxVI_String_Free();
esxVI_ObjectContent_Free();
-esxVI_DatastoreInfo_Free();
return result;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] esx: implement connectListAllStoragePools
Implement the .connectListAllStoragePools storage API in the esx storage
driver, and in all its subdrivers.
Signed-off-by: Pino Toscano
---
src/esx/esx_storage_backend_iscsi.c | 72 +
src/esx/esx_storage_backend_vmfs.c | 98 +
src/esx/esx_storage_driver.c| 68
3 files changed, 238 insertions(+)
diff --git a/src/esx/esx_storage_backend_iscsi.c
b/src/esx/esx_storage_backend_iscsi.c
index 72ab0d3cb0..4f5d8e5e24 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -779,6 +779,77 @@ esxStorageVolGetPath(virStorageVolPtr volume)
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllStoragePools(virConnectPtr conn,
+ virStoragePoolPtr **pools,
+ unsigned int flags)
+{
+bool success = false;
+size_t count = 0;
+esxPrivate *priv = conn->privateData;
+esxVI_HostInternetScsiHba *hostInternetScsiHba = NULL;
+esxVI_HostInternetScsiHbaStaticTarget *target;
+size_t i;
+
+/* this driver provides only iSCSI pools */
+if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE) &&
+!(MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI)))
+return 0;
+
+if (esxVI_LookupHostInternetScsiHba(priv->primary,
+) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to obtain iSCSI adapter"));
+goto cleanup;
+}
+
+/* FIXME: code looks for software iSCSI adapter only */
+if (!hostInternetScsiHba) {
+/* iSCSI adapter may not be enabled for this host */
+return 0;
+}
+
+/*
+ * ESX has two kind of targets:
+ * 1. staticIscsiTargets
+ * 2. dynamicIscsiTargets
+ * For each dynamic target if its reachable a static target is added.
+ * return iSCSI names for all static targets to avoid duplicate names.
+ */
+for (target = hostInternetScsiHba->configuredStaticTarget;
+ target; target = target->_next) {
+virStoragePoolPtr pool;
+
+pool = targetToStoragePool(conn, target->iScsiName, target);
+if (!pool)
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(*pools, count, pool) < 0)
+goto cleanup;
+}
+
+success = true;
+
+ cleanup:
+if (! success) {
+if (*pools) {
+for (i = 0; i < count; ++i)
+VIR_FREE((*pools)[i]);
+VIR_FREE(*pools);
+}
+
+count = -1;
+}
+
+esxVI_HostInternetScsiHba_Free();
+
+return count;
+}
+#undef MATCH
+
+
+
virStorageDriver esxStorageBackendISCSI = {
.connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 1.0.1 */
.connectListStoragePools = esxConnectListStoragePools, /* 1.0.1 */
@@ -799,4 +870,5 @@ virStorageDriver esxStorageBackendISCSI = {
.storageVolDelete = esxStorageVolDelete, /* 1.0.1 */
.storageVolWipe = esxStorageVolWipe, /* 1.0.1 */
.storageVolGetPath = esxStorageVolGetPath, /* 1.0.1 */
+.connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */
};
diff --git a/src/esx/esx_storage_backend_vmfs.c
b/src/esx/esx_storage_backend_vmfs.c
index b890825a40..05b273aed7 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -1460,6 +1460,103 @@ esxStorageVolGetPath(virStorageVolPtr volume)
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllStoragePools(virConnectPtr conn,
+ virStoragePoolPtr **pools,
+ unsigned int flags)
+{
+bool success = false;
+esxPrivate *priv = conn->privateData;
+esxVI_String *propertyNameList = NULL;
+esxVI_DynamicProperty *dynamicProperty = NULL;
+esxVI_ObjectContent *datastoreList = NULL;
+esxVI_ObjectContent *datastore = NULL;
+size_t count = 0;
+size_t i;
+virStoragePoolPtr pool;
+const bool checkPoolType =
MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE);
+
+if (esxVI_String_AppendValueToList(,
+ "summary.name") < 0) {
+goto cleanup;
+}
+
+if (checkPoolType &&
+esxVI_String_AppendValueToList(,
+ "info") < 0) {
+goto cleanup;
+}
+
+if (esxVI_LookupDatastoreList(priv->primary, propertyNameList,
+ ) < 0) {
+goto cleanup;
+}
+
+for (datastore = datastoreList; datastore;
+ datastore = datastore->_next) {
+const char *name = NULL;
+
+for (dynamicProperty = datastore->propSet; dynamicProperty;
+ dynamicProperty = dynamicProperty->_next) {
+if (STREQ(dynamicProperty->name, "summary.name")) {
+if (esxVI_AnyType_ExpectType(dynamicProperty->val,
+ esxVI_Type_String) < 0) {
+
[libvirt] [PATCH 0/5] esx: implement virConnectListAllStoragePools
Make the virConnectListAllStoragePools API work for esx storage pools by implementing the internal connectListAllStoragePools storage API. Pino Toscano (5): esx: split datastoreToStoragePoolPtr helper esx: split datastorePoolType helper esx: split targetToStoragePool helper esx: implement connectListAllStoragePools docs: document virConnectListAllStoragePools in esx docs/news.xml | 9 ++ src/esx/esx_storage_backend_iscsi.c | 104 +-- src/esx/esx_storage_backend_vmfs.c | 192 +++- src/esx/esx_storage_driver.c| 68 ++ 4 files changed, 332 insertions(+), 41 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] esx: split datastoreToStoragePoolPtr helper
Move the creation of a virStoragePtr object from the esxVI_ObjectContent
object of a datastore out of esxStoragePoolLookupByName in an own
helper. This way it can be used also in other functions.
Signed-off-by: Pino Toscano
---
src/esx/esx_storage_backend_vmfs.c | 45 --
1 file changed, 31 insertions(+), 14 deletions(-)
diff --git a/src/esx/esx_storage_backend_vmfs.c
b/src/esx/esx_storage_backend_vmfs.c
index 5f25f80072..78fe2b598d 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -195,26 +195,16 @@ esxConnectListStoragePools(virConnectPtr conn, char
**const names,
static virStoragePoolPtr
-esxStoragePoolLookupByName(virConnectPtr conn,
- const char *name)
+datastoreToStoragePoolPtr(virConnectPtr conn,
+ const char *name,
+ esxVI_ObjectContent *datastore)
{
esxPrivate *priv = conn->privateData;
-esxVI_ObjectContent *datastore = NULL;
esxVI_DatastoreHostMount *hostMount = NULL;
/* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
virStoragePoolPtr pool = NULL;
-if (esxVI_LookupDatastoreByName(priv->primary, name, NULL, ,
-esxVI_Occurrence_OptionalItem) < 0) {
-goto cleanup;
-}
-
-if (!datastore) {
-/* Not found, let the base storage driver handle error reporting */
-goto cleanup;
-}
-
/*
* Datastores don't have a UUID, but we can use the 'host.mountInfo.path'
* property as source for a UUID. The mount path is unique per host and
@@ -239,7 +229,6 @@ esxStoragePoolLookupByName(virConnectPtr conn,
pool = virGetStoragePool(conn, name, md5, , NULL);
cleanup:
-esxVI_ObjectContent_Free();
esxVI_DatastoreHostMount_Free();
return pool;
@@ -247,6 +236,34 @@ esxStoragePoolLookupByName(virConnectPtr conn,
+static virStoragePoolPtr
+esxStoragePoolLookupByName(virConnectPtr conn,
+ const char *name)
+{
+esxPrivate *priv = conn->privateData;
+esxVI_ObjectContent *datastore = NULL;
+virStoragePoolPtr pool = NULL;
+
+if (esxVI_LookupDatastoreByName(priv->primary, name, NULL, ,
+esxVI_Occurrence_OptionalItem) < 0) {
+goto cleanup;
+}
+
+if (!datastore) {
+/* Not found, let the base storage driver handle error reporting */
+goto cleanup;
+}
+
+pool = datastoreToStoragePoolPtr(conn, name, datastore);
+
+ cleanup:
+esxVI_ObjectContent_Free();
+
+return pool;
+}
+
+
+
static virStoragePoolPtr
esxStoragePoolLookupByUUID(virConnectPtr conn,
const unsigned char *uuid)
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] docs: document virConnectListAllStoragePools in esx
Signed-off-by: Pino Toscano --- docs/news.xml | 9 + 1 file changed, 9 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index c362bf3a15..278697506c 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -55,6 +55,15 @@ + + + esx driver: implement virConnectListAllStoragePools + + + The virConnectListAllStoragePools API was implemented in the esx + driver. + + -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] esx: split targetToStoragePool helper
Move the creation of a virStoragePtr object from the
esxVI_HostInternetScsiHbaStaticTarget object of a target out of
esxStoragePoolLookupByName in an own helper. This way it can be used
also in other functions.
Signed-off-by: Pino Toscano
---
src/esx/esx_storage_backend_iscsi.c | 32 +++--
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/src/esx/esx_storage_backend_iscsi.c
b/src/esx/esx_storage_backend_iscsi.c
index 61354a6938..72ab0d3cb0 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -147,14 +147,32 @@ esxConnectListStoragePools(virConnectPtr conn, char
**const names,
+static virStoragePoolPtr
+targetToStoragePool(virConnectPtr conn,
+const char *name,
+esxVI_HostInternetScsiHbaStaticTarget *target)
+{
+/* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
+unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
+
+/*
+ * HostInternetScsiHbaStaticTarget does not provide a uuid field,
+ * but iScsiName (or widely known as IQN) is unique across the multiple
+ * hosts, using it to compute key
+ */
+if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, target->iScsiName, md5) < 0)
+return NULL;
+
+return virGetStoragePool(conn, name, md5, , NULL);
+}
+
+
static virStoragePoolPtr
esxStoragePoolLookupByName(virConnectPtr conn,
const char *name)
{
esxPrivate *priv = conn->privateData;
esxVI_HostInternetScsiHbaStaticTarget *target = NULL;
-/* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
-unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
virStoragePoolPtr pool = NULL;
/*
@@ -172,15 +190,7 @@ esxStoragePoolLookupByName(virConnectPtr conn,
goto cleanup;
}
-/*
- * HostInternetScsiHbaStaticTarget does not provide a uuid field,
- * but iScsiName (or widely known as IQN) is unique across the multiple
- * hosts, using it to compute key
- */
-if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, target->iScsiName, md5) < 0)
-goto cleanup;
-
-pool = virGetStoragePool(conn, name, md5, , NULL);
+pool = targetToStoragePool(conn, name, target);
cleanup:
esxVI_HostInternetScsiHbaStaticTarget_Free();
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/9] Remove VIR_STRNDUP usage with checked pointers
This patch failed to compile for me with the following error:
../../tools/vsh.c:70:1: error: 'vshErrorOOM' defined but not used
[-Werror=unused-function]
70 | vshErrorOOM(void)
| ^~~
Apparently you removed the last call to vshErrorOOM() in the tools/vsh.c
changes here. For
reference, I am testing this series on top of this commit:
commit 54dd0938375daaa68674b271f444b312d6684b01 (origin/master, origin/HEAD,
master)
Author: Ján Tomko
Date: Wed Nov 13 09:51:45 2019 +0100
locking: fix build with older sanlock
Removing the function makes the patch compile without errors:
diff --git a/tools/vsh.c b/tools/vsh.c
index 2b1e0506ba..5005b1deaa 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -65,17 +65,6 @@ const vshCmdGrp *cmdGroups;
const vshCmdDef *cmdSet;
-/* simple handler for oom conditions */
-static void
-vshErrorOOM(void)
-{
-fflush(stdout);
-fputs(_("error: Out of memory\n"), stderr);
-fflush(stderr);
-exit(EXIT_FAILURE);
-}
-
-
double
vshPrettyCapacity(unsigned long long val, const char **unit)
{
Aside from this, patch LGTM.
Thanks,
DHB
On 11/12/19 2:02 PM, Ján Tomko wrote:
Remove the usage where sanity of the length argument is verified
by other conditions not matching the previous patches.
Signed-off-by: Ján Tomko
---
src/libxl/xen_common.c | 18 --
tools/vsh.c| 5 ++---
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index a4a9ec59bf..7ac24fb606 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -817,9 +817,8 @@ xenParseSxprChar(const char *value,
goto error;
}
-if (offset != value &&
-VIR_STRNDUP(def->source->data.tcp.host, value, offset - value) < 0)
-goto error;
+if (offset != value)
+def->source->data.tcp.host = g_strndup(value, offset - value);
offset2 = strchr(offset, ',');
offset++;
@@ -845,9 +844,9 @@ xenParseSxprChar(const char *value,
goto error;
}
-if (offset != value &&
-VIR_STRNDUP(def->source->data.udp.connectHost, value, offset - value)
< 0)
-goto error;
+if (offset != value)
+def->source->data.udp.connectHost = g_strndup(value,
+ offset - value);
offset2 = strchr(offset, '@');
if (offset2 != NULL) {
@@ -862,10 +861,9 @@ xenParseSxprChar(const char *value,
goto error;
}
-if (offset3 > (offset2 + 1) &&
-VIR_STRNDUP(def->source->data.udp.bindHost,
-offset2 + 1, offset3 - offset2 - 1) < 0)
-goto error;
+if (offset3 > (offset2 + 1))
+def->source->data.udp.bindHost = g_strndup(offset2 + 1,
+ offset3 - offset2 -
1);
def->source->data.udp.bindService = g_strdup(offset3 + 1);
} else {
diff --git a/tools/vsh.c b/tools/vsh.c
index 000cf6a009..2b1e0506ba 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -360,9 +360,8 @@ vshCmddefCheckInternals(vshControl *ctl,
cmd->name);
return -1; /* alias options are tracked by the original name
*/
}
-if ((p = strchr(name, '=')) &&
-VIR_STRNDUP(name, name, p - name) < 0)
-vshErrorOOM();
+if ((p = strchr(name, '=')))
+name = g_strndup(name, p - name);
for (j = i + 1; cmd->opts[j].name; j++) {
if (STREQ(name, cmd->opts[j].name) &&
cmd->opts[j].type != VSH_OT_ALIAS)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/9] Remove VIR_STRNDUP usage that subtracts from a non-NULL pointer
On 11/12/19 2:02 PM, Ján Tomko wrote: Use g_strndup in all the cases where we check upfront whether a pointer is non-NULL and then use it to calculate the copied length. Signed-off-by: Ján Tomko --- src/util/virsysinfo.c | 241 -- 1 file changed, 112 insertions(+), 129 deletions(-) Reviewed-by: Daniel Henrique Barboza -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/9] Remove VIR_STRNDUP usage that passes -1
On 11/12/19 2:02 PM, Ján Tomko wrote:
Replace all the usage of
VIR_STRNDUP(dest, b, p ? p - b : -1)
with separate calls to g_strndup/g_strdup.
Signed-off-by: Ján Tomko
---
src/bhyve/bhyve_parse_command.c | 38 +
src/libxl/xen_common.c | 15 +++--
src/remote/remote_driver.c | 6 --
3 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
index fa3b881f21..5c5dfae4a9 100644
--- a/src/bhyve/bhyve_parse_command.c
+++ b/src/bhyve/bhyve_parse_command.c
@@ -150,8 +150,10 @@ bhyveCommandLineToArgv(const char *nativeConfig,
start = curr;
next = strchr(curr, '\n');
-if (VIR_STRNDUP(line, curr, next ? next - curr : -1) < 0)
-goto error;
+if (next)
+line = g_strndup(curr, next - curr);
+else
+line = g_strdup(curr);
if (VIR_RESIZE_N(lines, lines_alloc, line_count, 2) < 0) {
VIR_FREE(line);
@@ -194,8 +196,10 @@ bhyveCommandLineToArgv(const char *nativeConfig,
next = strchr(start, ' ');
}
-if (VIR_STRNDUP(arg, curr, next ? next - curr : -1) < 0)
-goto error;
+if (next)
+arg = g_strndup(curr, next - curr);
+else
+arg = g_strdup(curr);
if (next && (*next == '\'' || *next == '"'))
next++;
@@ -366,8 +370,10 @@ bhyveParsePCISlot(const char *slotdef,
next = strchr(curr, ':');
- if (VIR_STRNDUP(val, curr, next? next - curr : -1) < 0)
- goto error;
+ if (next)
+ val = g_strndup(curr, next - curr);
+ else
+ val = g_strdup(curr);
if (virStrToLong_ui(val, NULL, 10, [i]) < 0)
goto error;
@@ -441,9 +447,10 @@ bhyveParsePCIDisk(virDomainDefPtr def,
goto error;
separator = strchr(config, ',');
-if (VIR_STRNDUP(disk->src->path, config,
-separator? separator - config : -1) < 0)
-goto error;
+if (separator)
+disk->src->path = g_strndup(config, separator - config);
+else
+disk->src->path = g_strdup(config);
if (bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
idx = *nvirtiodisk;
@@ -515,9 +522,10 @@ bhyveParsePCINet(virDomainDefPtr def,
}
separator = strchr(config, ',');
-if (VIR_STRNDUP(net->ifname, config,
-separator? separator - config : -1) < 0)
-goto error;
+if (separator)
+net->ifname = g_strndup(config, separator - config);
+else
+net->ifname = g_strdup(config);
if (!separator)
goto cleanup;
@@ -578,8 +586,10 @@ bhyveParseBhyvePCIArg(virDomainDefPtr def,
if (conf)
conf++; /* Skip initial comma */
-if (VIR_STRNDUP(emulation, separator, conf? conf - separator - 1 : -1) < 0)
-goto error;
+if (conf)
+emulation = g_strndup(separator, conf - separator - 1);
+else
+emulation = g_strdup(separator);
Nit: there's an 'if (conf)' right before the VIR_STRNDUP() call you're
replacing.
You can get a ride in it to insert the g_strndup call instead of adding a new
'if (conf)':
if (conf) {
conf++; /* Skip initial comma */
emulation = g_strndup(separator, conf - separator - 1);
} else {
emulation = g_strdup(separator);
}
Reviewed-by: Daniel Henrique Barboza
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: use GUINT32_SWAP_LE_BE
On Wed, Nov 13, 2019 at 12:44:57PM +0100, Peter Krempa wrote: > On Wed, Nov 13, 2019 at 10:38:06 +, Daniel Berrange wrote: > > On Wed, Nov 13, 2019 at 10:33:09AM +0100, Ján Tomko wrote: > > > Use this GLib macro instead of bswap_32 from gnulib. > > > > > > Signed-off-by: Ján Tomko > > > --- > > > src/qemu/qemu_driver.c | 10 +- > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > Missing bootstrap.conf change to remove byteswap > > We had a chat in person and we agreed to try to get rid of as much of > the modules as possible by doing a "kill one gnulib module a day" > initiative at the office. FYI, most(all?) of the gnulib modules related to sockets/FDs will need to be killed off in a single group, since they're all intertwined to deal with the Windows HANDLE vs FD wrapping GNULIB does. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/9] Remove VIR_STRDUP usage that snuck in
On 11/12/19 2:02 PM, Ján Tomko wrote: Fixes: 224d269f19f0a6c496dd2218f934a54742d51708 Signed-off-by: Ján Tomko --- src/qemu/qemu_domain.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) Reviewed-by: Daniel Henrique Barboza -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: use GUINT32_SWAP_LE_BE
On Wed, Nov 13, 2019 at 10:38:06 +, Daniel Berrange wrote:
> On Wed, Nov 13, 2019 at 10:33:09AM +0100, Ján Tomko wrote:
> > Use this GLib macro instead of bswap_32 from gnulib.
> >
> > Signed-off-by: Ján Tomko
> > ---
> > src/qemu/qemu_driver.c | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
>
> Missing bootstrap.conf change to remove byteswap
We had a chat in person and we agreed to try to get rid of as much of
the modules as possible by doing a "kill one gnulib module a day"
initiative at the office.
Since bootstrap changes are very invasive for incremental builds I
pledge that I'll collect them in a branch and push all outstanding ones
once a week to minimize bootstrap churn. I'll specify the branch in my
patches.
Thus bootstrap.conf changes should be kept separate.
>
>
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index c969a3d463..85b0c9cb79 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -2803,11 +2803,11 @@ struct _virQEMUSaveData {
> > static inline void
> > bswap_header(virQEMUSaveHeaderPtr hdr)
> > {
> > -hdr->version = bswap_32(hdr->version);
> > -hdr->data_len = bswap_32(hdr->data_len);
> > -hdr->was_running = bswap_32(hdr->was_running);
> > -hdr->compressed = bswap_32(hdr->compressed);
> > -hdr->cookieOffset = bswap_32(hdr->cookieOffset);
> > +hdr->version = GUINT32_SWAP_LE_BE(hdr->version);
> > +hdr->data_len = GUINT32_SWAP_LE_BE(hdr->data_len);
> > +hdr->was_running = GUINT32_SWAP_LE_BE(hdr->was_running);
> > +hdr->compressed = GUINT32_SWAP_LE_BE(hdr->compressed);
> > +hdr->cookieOffset = GUINT32_SWAP_LE_BE(hdr->cookieOffset);
> > }
>
> Also needs to remove byteswap.h include file
This can be done in this patch though.
>
>
> 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 :|
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [rust PATCH] api_tests.py: Fix all issues reported by flake8
On Wed, Nov 13, 2019 at 12:03:56PM +0100, Andrea Bolognani wrote: > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [rust PATCH] api_tests.py: Fix all issues reported by flake8
Signed-off-by: Andrea Bolognani
---
tools/api_tests.py | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/tools/api_tests.py b/tools/api_tests.py
index 9e66c92..7f0e3cb 100644
--- a/tools/api_tests.py
+++ b/tools/api_tests.py
@@ -6,36 +6,39 @@ import xml.etree.ElementTree
import sys
-LIBVIRT_API_FILE="/usr/share/libvirt/api/libvirt-api.xml"
+LIBVIRT_API_FILE = "/usr/share/libvirt/api/libvirt-api.xml"
MY_PATH = os.path.dirname(os.path.realpath(__file__))
SRC_PATH = MY_PATH + "/../src"
+
def get_api_symbols(doc):
funcs = doc.iter('function')
macros = doc.iter('macro')
enums = doc.iter('enum')
return funcs, macros, enums
+
def get_sources():
return glob.glob(SRC_PATH + "/*.rs")
+
def match(el, content):
return content.find(el) >= 0
+
def main():
filter_by = ""
if len(sys.argv) > 1:
filter_by = sys.argv[1]
-
+
doc = xml.etree.ElementTree.parse(LIBVIRT_API_FILE).getroot()
implemented = set([])
missing = set([])
for el in doc.iter('function'):
-
-if el.get('name').startswith(filter_by): # What i'm looking for?
-
+if el.get('name').startswith(filter_by): # What I'm looking for
+
status = False
for source in get_sources():
f = open(source)
@@ -51,13 +54,10 @@ def main():
print("missing:")
for x in missing:
print(x.attrib)
-#print "implemented:"
-#for x in implemented:
-#print x.attrib
-
+# print "implemented:"
+# for x in implemented:
+# print x.attrib
+
if __name__ == '__main__':
main()
-
-
-
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: use GUINT32_SWAP_LE_BE
On Wed, Nov 13, 2019 at 10:33:09AM +0100, Ján Tomko wrote:
> Use this GLib macro instead of bswap_32 from gnulib.
>
> Signed-off-by: Ján Tomko
> ---
> src/qemu/qemu_driver.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
Missing bootstrap.conf change to remove byteswap
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c969a3d463..85b0c9cb79 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2803,11 +2803,11 @@ struct _virQEMUSaveData {
> static inline void
> bswap_header(virQEMUSaveHeaderPtr hdr)
> {
> -hdr->version = bswap_32(hdr->version);
> -hdr->data_len = bswap_32(hdr->data_len);
> -hdr->was_running = bswap_32(hdr->was_running);
> -hdr->compressed = bswap_32(hdr->compressed);
> -hdr->cookieOffset = bswap_32(hdr->cookieOffset);
> +hdr->version = GUINT32_SWAP_LE_BE(hdr->version);
> +hdr->data_len = GUINT32_SWAP_LE_BE(hdr->data_len);
> +hdr->was_running = GUINT32_SWAP_LE_BE(hdr->was_running);
> +hdr->compressed = GUINT32_SWAP_LE_BE(hdr->compressed);
> +hdr->cookieOffset = GUINT32_SWAP_LE_BE(hdr->cookieOffset);
> }
Also needs to remove byteswap.h include file
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] util: use g_vsnprintf
On Wed, Nov 13, 2019 at 10:33:08AM +0100, Ján Tomko wrote: > Instead of vsnprintf from gnulib, use g_vsnprintf from GLib. > > Signed-off-by: Ján Tomko > --- > src/util/virerror.c | 4 ++-- > src/util/virtypedparam.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) The change to bootsrap.conf is missing to remove vsnprintf. With that added 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] conf: showing cache/memoryBW monitor features in capabilities
From: Huaqiang
We learned that the hardware features of CAT, CMT, MBA and MBM
are orthogonal ones, if CAT or MBA is not supported in system,
but CMT or MBM are supported, then the cache monitor or
memoryBW monitor features may not be correctly displayed in
host capabilities through command 'virsh capabilites'.
Showing the cache/memoryBW monitor capabilities even there is
no support of cache allocation or memoryBW allocation features.
Signed-off-by: Huaqiang
---
src/conf/capabilities.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 7edec75c31..a048427241 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -939,7 +939,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
size_t i = 0;
size_t j = 0;
-if (!cache->nbanks)
+if (!cache->nbanks && !cache->monitor)
return 0;
virBufferAddLit(buf, "\n");
@@ -1025,7 +1025,7 @@ virCapabilitiesFormatMemoryBandwidth(virBufferPtr buf,
{
size_t i = 0;
-if (!memBW->nnodes)
+if (!memBW->nnodes && !memBW->monitor)
return 0;
virBufferAddLit(buf, "\n");
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] virsh: show memoryBW info in 'virsh domstats' command
From: Huaqiang
Introduce an option '--memory' for showing memory related
information. The memory bandwidth infomatio is listed as:
Domain: 'libvirt-vm'
memory.bandwidth.monitor.count=4
memory.bandwidth.monitor.0.name=vcpus_0-4
memory.bandwidth.monitor.0.vcpus=0-4
memory.bandwidth.monitor.0.node.count=2
memory.bandwidth.monitor.0.node.0.id=0
memory.bandwidth.monitor.0.node.0.bytes.total=10208067584
memory.bandwidth.monitor.0.node.0.bytes.local=4807114752
memory.bandwidth.monitor.0.node.1.id=1
memory.bandwidth.monitor.0.node.1.bytes.total=8693735424
memory.bandwidth.monitor.0.node.1.bytes.local=5850161152
memory.bandwidth.monitor.1.name=vcpus_7
memory.bandwidth.monitor.1.vcpus=7
memory.bandwidth.monitor.1.node.count=2
memory.bandwidth.monitor.1.node.0.id=0
memory.bandwidth.monitor.1.node.0.bytes.total=853811200
memory.bandwidth.monitor.1.node.0.bytes.local=290701312
memory.bandwidth.monitor.1.node.1.id=1
memory.bandwidth.monitor.1.node.1.bytes.total=406044672
memory.bandwidth.monitor.1.node.1.bytes.local=229425152
Signed-off-by: Huaqiang
---
include/libvirt/libvirt-domain.h | 1 +
src/libvirt-domain.c | 21 +++
src/qemu/qemu_driver.c | 99
tools/virsh-domain-monitor.c | 7 +++
tools/virsh.pod | 23 +++-
5 files changed, 149 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 22277b0a84..2b621ff162 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2146,6 +2146,7 @@ typedef enum {
VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */
VIR_DOMAIN_STATS_PERF = (1 << 6), /* return domain perf event info */
VIR_DOMAIN_STATS_IOTHREAD = (1 << 7), /* return iothread poll info */
+VIR_DOMAIN_STATS_MEMORY= (1 << 8), /* return domain memory info */
} virDomainStatsTypes;
typedef enum {
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index dcab179e6e..c8c543ccde 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11641,6 +11641,27 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* hypervisor to choose how to shrink the
* polling time.
*
+ * VIR_DOMAIN_STATS_MEMORY:
+ * Return memory bandwidth statistics and the usage information. The typed
+ * parameter keys are in this format:
+ *
+ * "memory.bandwidth.monitor.count" - the number of memory bandwidth
+ *monitors for this domain
+ * "memory.bandwidth.monitor..name" - the name of monitor
+ * "memory.bandwidth.monitor..vcpus" - the vcpu list of monitor
+ * "memory.bandwidth.monitor..node.count" - the number of memory
+ *controller in monitor
+ * "memory.bandwidth.monitor..node..id" - host allocated memory
+ * controller id for controller
+ * of monitor
+ * "memory.bandwidth.monitor..node..bytes.local" - the
+ * accumulative bytes consumed by @vcpus that passing
+ * through the memory controller in the same processor
+ * that the scheduled host CPU belongs to.
+ * "memory.bandwidth.monitor..node..bytes.total" - the total
+ * bytes consumed by @vcpus that passing through all
+ * memory controllers, either local or remote controller.
+ *
* Note that entire stats groups or individual stat fields may be missing from
* the output in case they are not supported by the given hypervisor, are not
* applicable for the current state of the guest domain, or their retrieval
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e396358871..37a986a1bd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20496,6 +20496,9 @@ qemuDomainGetResctrlMonData(virQEMUDriverPtr driver,
features = caps->host.cache.monitor->features;
break;
case VIR_RESCTRL_MONITOR_TYPE_MEMBW:
+if (caps->host.memBW.monitor)
+features = caps->host.memBW.monitor->features;
+break;
case VIR_RESCTRL_MONITOR_TYPE_UNSUPPORT:
case VIR_RESCTRL_MONITOR_TYPE_LAST:
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
@@ -20548,6 +20551,90 @@ qemuDomainGetResctrlMonData(virQEMUDriverPtr driver,
}
+static int
+qemuDomainGetStatsMemoryBandwidth(virQEMUDriverPtr driver,
+ virDomainObjPtr dom,
+ virTypedParamListPtr params)
+{
+virQEMUResctrlMonDataPtr *resdata = NULL;
+char **features = NULL;
+size_t nresdata = 0;
+size_t i = 0;
+size_t j = 0;
+size_t k = 0;
+int ret = -1;
+
+if (!virDomainObjIsActive(dom))
+return 0;
+
+if
[libvirt] [PATCH 4/5] conf: Parse dommon configure file for memorytune monitors
From: Huaqiang
Create memory bandwidth monitor.
Following domain configuration changes create two memory bandwidth
monitors: one is monitoring the bandwidth consumed by vCPU 0,
another is for vCPU 5.
```
+
+
+
+
```
Signed-off-by: Huaqiang
---
docs/schemas/domaincommon.rng | 23 +++
src/conf/domain_conf.c | 44 +-
tests/genericxml2xmlindata/memorytune.xml | 5 +++
tests/genericxml2xmloutdata/memorytune.xml | 42 +
tests/genericxml2xmltest.c | 2 +-
5 files changed, 98 insertions(+), 18 deletions(-)
create mode 100644 tests/genericxml2xmloutdata/memorytune.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index aa4f512e5c..adf1fcb36d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1023,14 +1023,21 @@
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 875490dd2b..51bf18a42b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19639,10 +19639,14 @@ virDomainMemorytuneDefParse(virDomainDefPtr def,
{
VIR_XPATH_NODE_AUTORESTORE(ctxt);
virDomainResctrlDefPtr resctrl = NULL;
+virDomainResctrlDefPtr newresctrl = NULL;
g_autoptr(virBitmap) vcpus = NULL;
g_autofree xmlNodePtr *nodes = NULL;
g_autoptr(virResctrlAlloc) alloc = NULL;
ssize_t i = 0;
+size_t nmons = 0;
+size_t ret = -1;
+
int n;
ctxt->node = node;
@@ -19669,29 +19673,44 @@ virDomainMemorytuneDefParse(virDomainDefPtr def,
return -1;
}
+/* First, parse element if any element exists */
for (i = 0; i < n; i++) {
if (virDomainMemorytuneDefParseMemory(ctxt, nodes[i], alloc) < 0)
return -1;
}
-if (n == 0)
-return 0;
-
/*
* If this is a new allocation, format ID and append to resctrl, otherwise
* just update the existing alloc information, which is done in above
* virDomainMemorytuneDefParseMemory */
if (!resctrl) {
-if (!(resctrl = virDomainResctrlNew(node, alloc, vcpus, flags)))
+if (!(newresctrl = virDomainResctrlNew(node, alloc, vcpus, flags)))
return -1;
-if (VIR_APPEND_ELEMENT(def->resctrls, def->nresctrls, resctrl) < 0) {
-virDomainResctrlDefFree(resctrl);
-return -1;
-}
+resctrl = newresctrl;
}
-return 0;
+/* Next, parse element */
+nmons = resctrl->nmonitors;
+if (virDomainResctrlMonDefParse(def, ctxt, node,
+VIR_RESCTRL_MONITOR_TYPE_MEMBW,
+resctrl) < 0)
+goto cleanup;
+
+nmons = resctrl->nmonitors - nmons;
+/* Now @nmons contains the new element number found in current
+ * element, and @n holds the number of new element,
+ * only append the new @newresctrl object to domain if any of them is
+ * not zero. */
+if (newresctrl && (nmons || n)) {
+if (VIR_APPEND_ELEMENT(def->resctrls, def->nresctrls, newresctrl) < 0)
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+virDomainResctrlDefFree(newresctrl);
+return ret;
}
@@ -27607,6 +27626,7 @@ virDomainMemorytuneDefFormat(virBufferPtr buf,
{
g_auto(virBuffer) childrenBuf = VIR_BUFFER_INITIALIZER;
g_autofree char *vcpus = NULL;
+size_t i = 0;
virBufferSetChildIndent(, buf);
if (virResctrlAllocForeachMemory(resctrl->alloc,
@@ -27614,6 +27634,12 @@ virDomainMemorytuneDefFormat(virBufferPtr buf,
) < 0)
return -1;
+for (i = 0; i< resctrl->nmonitors; i++) {
+if (virDomainResctrlMonDefFormatHelper(resctrl->monitors[i],
+ VIR_RESCTRL_MONITOR_TYPE_MEMBW,
+ ) < 0)
+return -1;
+}
if (!virBufferUse())
return 0;
diff --git a/tests/genericxml2xmlindata/memorytune.xml
b/tests/genericxml2xmlindata/memorytune.xml
index 7486b542c5..8d86ce4282 100644
--- a/tests/genericxml2xmlindata/memorytune.xml
+++ b/tests/genericxml2xmlindata/memorytune.xml
@@ -10,12 +10,17 @@
+
+
+
[libvirt] [PATCH 3/5] cachetune schema: a looser check for the order of and element
From: Huaqiang
Originally, inside , it requires the element to
be in the position before , and following configuration is not
permitted by schema, but it is better to let it be valid.
^
|__ Not permitted originally because it is in the place
before element.
...
And, let schema do more strict check by identifying following configuration to
be invalid, due to should contain at least one or
element.
^
|__ a SHOULD contain at least one or
...
Signed-off-by: Huaqiang
---
docs/schemas/domaincommon.rng | 68 +++
tests/genericxml2xmlindata/cachetune.xml | 1 +
tests/genericxml2xmloutdata/cachetune.xml | 34
tests/genericxml2xmltest.c| 2 +-
4 files changed, 70 insertions(+), 35 deletions(-)
create mode 100644 tests/genericxml2xmloutdata/cachetune.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index e06f892da3..aa4f512e5c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -980,41 +980,41 @@
-
-
-
-
-
-
-
-
-
-
-both
-code
-data
-
-
-
-
-
-
-
-
+
+
+
+
+
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+ both
+ code
+ data
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/genericxml2xmlindata/cachetune.xml
b/tests/genericxml2xmlindata/cachetune.xml
index 645cab7771..eda2ca6fb6 100644
--- a/tests/genericxml2xmlindata/cachetune.xml
+++ b/tests/genericxml2xmlindata/cachetune.xml
@@ -6,6 +6,7 @@
4
+
diff --git a/tests/genericxml2xmloutdata/cachetune.xml
b/tests/genericxml2xmloutdata/cachetune.xml
new file mode 100644
index 00..dcde0ebc2a
--- /dev/null
+++ b/tests/genericxml2xmloutdata/cachetune.xml
@@ -0,0 +1,34 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 4
+
+
+
+
+
+
+
+
+
+
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-i686
+
+
+
+
+
+
+
+
diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c
index 0d04413712..62005a5393 100644
--- a/tests/genericxml2xmltest.c
+++ b/tests/genericxml2xmltest.c
@@ -125,9 +125,9 @@ mymain(void)
DO_TEST_FULL("chardev-reconnect-invalid-mode", 0, false,
TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE);
-DO_TEST("cachetune");
DO_TEST("cachetune-small");
DO_TEST("cachetune-cdp");
+DO_TEST_DIFFERENT("cachetune");
DO_TEST_DIFFERENT("cachetune-extra-tunes");
DO_TEST_FULL("cachetune-colliding-allocs", false, true,
TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE);
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] util, resctrl: using 64bit interface instead of 32bit for counters
From: Huaqiang
The underlying resctrl monitoring is actually using 64 bit counters,
not the 32bit one. Correct this by using 64bit interfaces.
Signed-off-by: Huaqiang
---
src/qemu/qemu_driver.c | 4 ++--
src/util/virfile.c | 40
src/util/virfile.h | 2 ++
src/util/virresctrl.c | 6 +++---
src/util/virresctrl.h | 2 +-
5 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f4ff2ba292..e396358871 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20587,8 +20587,8 @@ qemuDomainGetStatsCpuCache(virQEMUDriverPtr driver,
"cpu.cache.monitor.%zu.bank.%zu.id",
i, j) < 0)
goto cleanup;
-if (virTypedParamListAddUInt(params, resdata[i]->stats[j]->vals[0],
-
"cpu.cache.monitor.%zu.bank.%zu.bytes", i, j) < 0)
+if (virTypedParamListAddULLong(params,
resdata[i]->stats[j]->vals[0],
+
"cpu.cache.monitor.%zu.bank.%zu.bytes", i, j) < 0)
goto cleanup;
}
}
diff --git a/src/util/virfile.c b/src/util/virfile.c
index ced0ea70b7..372498e69e 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -4204,6 +4204,46 @@ virFileReadValueUint(unsigned int *value, const char
*format, ...)
}
+/**
+ * virFileReadValueUllong:
+ * @value: pointer to unsigned long long to be filled in with the value
+ * @format, ...: file to read from
+ *
+ * Read unsigned int from @format and put it into @value.
+ *
+ * Return -2 for non-existing file, -1 on other errors and 0 if everything went
+ * fine.
+ */
+int
+virFileReadValueUllong(unsigned long long *value, const char *format, ...)
+{
+g_autofree char *str = NULL;
+g_autofree char *path = NULL;
+va_list ap;
+
+va_start(ap, format);
+path = g_strdup_vprintf(format, ap);
+va_end(ap);
+
+if (!virFileExists(path))
+return -2;
+
+if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), ) < 0)
+return -1;
+
+virStringTrimOptionalNewline(str);
+
+if (virStrToLong_ullp(str, NULL, 10, value) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid unsigned long long value '%s' in file '%s'"),
+ str, path);
+return -1;
+}
+
+return 0;
+}
+
+
/**
* virFileReadValueScaledInt:
* @value: pointer to unsigned long long int to be filled in with the value
diff --git a/src/util/virfile.h b/src/util/virfile.h
index a570529330..f77d8501c3 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -363,6 +363,8 @@ int virFileReadValueInt(int *value, const char *format, ...)
G_GNUC_PRINTF(2, 3);
int virFileReadValueUint(unsigned int *value, const char *format, ...)
G_GNUC_PRINTF(2, 3);
+int virFileReadValueUllong(unsigned long long *value, const char *format, ...)
+ G_GNUC_PRINTF(2, 3);
int virFileReadValueBitmap(virBitmapPtr *value, const char *format, ...)
G_GNUC_PRINTF(2, 3);
int virFileReadValueScaledInt(unsigned long long *value, const char *format,
...)
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index b78fded026..684d2ce398 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -2678,7 +2678,7 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
int rv = -1;
int ret = -1;
size_t i = 0;
-unsigned int val = 0;
+unsigned long long val = 0;
DIR *dirp = NULL;
char *datapath = NULL;
char *filepath = NULL;
@@ -2734,8 +2734,8 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
goto cleanup;
for (i = 0; resources[i]; i++) {
-rv = virFileReadValueUint(, "%s/%s/%s", datapath,
- ent->d_name, resources[i]);
+rv = virFileReadValueUllong(, "%s/%s/%s", datapath,
+ent->d_name, resources[i]);
if (rv == -2) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("File '%s/%s/%s' does not exist."),
diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
index 3dd7c96348..11f275acf4 100644
--- a/src/util/virresctrl.h
+++ b/src/util/virresctrl.h
@@ -204,7 +204,7 @@ struct _virResctrlMonitorStats {
char **features;
/* @vals store the statistical record values and @val[0] is the value for
* @features[0], @val[1] for@features[1] ... respectively */
-unsigned int *vals;
+unsigned long long *vals;
/* The length of @vals array */
size_t nvals;
};
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH Rust v2] api_tests.py: update to use Python 3
From: Zixing Liu
Signed-off-by: Zixing Liu
---
tools/api_tests.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/api_tests.py b/tools/api_tests.py
index b26ec34..9e66c92 100644
--- a/tools/api_tests.py
+++ b/tools/api_tests.py
@@ -47,10 +47,10 @@ def main():
else:
missing.add(el)
-print "missing: %s, implemented: %s" % (len(missing), len(implemented))
-print "missing:"
+print("missing: %s, implemented: %s" % (len(missing), len(implemented)))
+print("missing:")
for x in missing:
-print x.attrib
+print(x.attrib)
#print "implemented:"
#for x in implemented:
#print x.attrib
--
2.24.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
