Re: [libvirt] [PATCH v2 1/4] qemu: block: propagate the delete flag to where it can actually be used
On Wed, Nov 20, 2019 at 18:12:18 +0100, Pavel Mores wrote:
> On Wed, Nov 20, 2019 at 12:14:11PM +0100, Peter Krempa wrote:
> > On Wed, Nov 20, 2019 at 11:44:52 +0100, Pavel Mores wrote:
> > > Since the blockcommit operation is asynchronous, this has conceptually two
> > > parts. First, we have to propagate the flag from qemuDomainBlockCommit()
> > > (which was just ignoring it until now) to qemuBlockJobDiskNewCommit().
> > > Then
> > > it can be stored in the qemuBlockJobCommitData structure which holds
> > > information necessary to finish the job asynchronously.
> > >
> > > Signed-off-by: Pavel Mores
> > > ---
> > > src/qemu/qemu_blockjob.c | 4 +++-
> > > src/qemu/qemu_blockjob.h | 4 +++-
> > > src/qemu/qemu_driver.c | 5 +++--
> > > 3 files changed, 9 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> > > index 5c294f8024..7d94a6ce38 100644
> > > --- a/src/qemu/qemu_blockjob.c
> > > +++ b/src/qemu/qemu_blockjob.c
> > > @@ -250,7 +250,8 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
> > >virDomainDiskDefPtr disk,
> > >virStorageSourcePtr topparent,
> > >virStorageSourcePtr top,
> > > - virStorageSourcePtr base)
> > > + virStorageSourcePtr base,
> > > + bool delete_imgs)
> > > {
> > > qemuDomainObjPrivatePtr priv = vm->privateData;
> > > g_autoptr(qemuBlockJobData) job = NULL;
> > > @@ -273,6 +274,7 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
> > > job->data.commit.topparent = topparent;
> > > job->data.commit.top = top;
> > > job->data.commit.base = base;
> > > +job->data.commit.deleteCommittedImages = delete_imgs;
> > >
> > > if (qemuBlockJobRegister(job, vm, disk, true) < 0)
> > > return NULL;
> > > diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
> > > index d8da918f2f..d77f1dcdb3 100644
> > > --- a/src/qemu/qemu_blockjob.h
> > > +++ b/src/qemu/qemu_blockjob.h
> > > @@ -85,6 +85,7 @@ struct _qemuBlockJobCommitData {
> > > virStorageSourcePtr topparent;
> > > virStorageSourcePtr top;
> > > virStorageSourcePtr base;
> > > +bool deleteCommittedImages;
> > > };
> > >
> > >
> > > @@ -165,7 +166,8 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
> > >virDomainDiskDefPtr disk,
> > >virStorageSourcePtr topparent,
> > >virStorageSourcePtr top,
> > > - virStorageSourcePtr base);
> > > + virStorageSourcePtr base,
> > > + bool delete_imgs);
> > >
> > > qemuBlockJobDataPtr
> > > qemuBlockJobNewCreate(virDomainObjPtr vm,
> > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > > index dc14ec86a3..75458f5c8a 100644
> > > --- a/src/qemu/qemu_driver.c
> > > +++ b/src/qemu/qemu_driver.c
> > > @@ -18496,10 +18496,10 @@ qemuDomainBlockCommit(virDomainPtr dom,
> > > bool persistjob = false;
> > > bool blockdev = false;
> > >
> > > -/* XXX Add support for COMMIT_DELETE */
> > > virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
> > >VIR_DOMAIN_BLOCK_COMMIT_ACTIVE |
> > >VIR_DOMAIN_BLOCK_COMMIT_RELATIVE |
> > > + VIR_DOMAIN_BLOCK_COMMIT_DELETE |
> > >VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES, -1);
> > >
> > > if (!(vm = qemuDomainObjFromDomain(dom)))
> > > @@ -18638,7 +18638,8 @@ qemuDomainBlockCommit(virDomainPtr dom,
> > > goto endjob;
> > >
> > > if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent,
> > > topSource,
> > > - baseSource)))
> > > + baseSource,
> > > + flags &
> > > VIR_DOMAIN_BLOCK_COMMIT_DELETE)))
> > > goto endjob;
> >
> > I'd prefer if these last two hunks which enable the feature are in a
> > separate commit at the end of the series, so that they enable it only
> > once all the plumbing is in place.
>
> I tried to figure out how to do this but I'm afraid I don't see a very good
> way. Here's my thinking: since code in patch 2 reads and uses
> qemuBlockJobCommitData.deleteCommittedImages, patch 1 should make sure it's
> there and it's initialised. So patch 1 should add deleteCommittedImages to
> qemuBlockJobCommitData and qemuBlockJobDiskNewCommit() should set it to a
> value. To get that value, it needs to take the extra argument. However, if
> it does take the extra argument, qemuDomainBlockCommit() has to pass it.
Just pass 'false' as the last argument of qemuBlockJobDiskNewCommit
which will equal to the same behaviour as it did until now.
Then pass the correct value along with enabling it in virCheckFlags
after the code which actually uses the value is
Re: [libvirt] [PATCH 0/4] apparmor fixes triggered by multi disk snapshots
On Thu, Nov 14, 2019 at 12:23 PM Christian Ehrhardt wrote: > > On Thu, Nov 14, 2019 at 1:23 AM Cole Robinson wrote: > > > > 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 > > Thank you, > I added the tag in my local series, but that is not worth a v2 submission. > Before pushing I'm still waiting for someone with apparmor experience > to take a look, just to be somewhat on the safe side. Thanks Jamie for also adding Review and Discusions. Pushing this with your Ack/Review tags after a final build/check > > - Cole > > -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/3] virt-aa-helper for shmem device
On Thu, Nov 14, 2019 at 12:20 PM Christian Ehrhardt wrote: > > Cole was recently adding a few of the usual apparmor suspects to BZ 1761645 > and I was taking a look at the low hanging fruits of it today. It isn't > perfect, but would resolve the reported issue - so I'd appreciate a > review. > > Limitations: > - One could break the path creating elements in qemuBuildShmemBackendMemProps > and qemuDomainPrepareShmemChardev into extra functions and then use those > from virt-aa-helper. But I haven't done so yet and unless it is strictly > required consider it too much for what we want/need to achieve here. > - I haven't covered hotplug of shmem devices yet, it seems there is no > infrastructure for their labels yet and I wasn't sure how important > shmem-hotplug would even be to anyone. > > Updates in v2: > - rebase latest master > - drop checking shmems[i] > - switch to use g_strdup_printf > - added an extra patch removing checks like ctl->def->mems[i] > - add reviewed-by tag Thanks everyone for the further review and discussion. Pushing with Ack/Review Tags after a rebuild/recheck > Christian Ehrhardt (3): > virt-aa-helper: add rules for shmem devices > virt-aa-helper: testcase for shmem devices > virt-aa-helper: drop pointer checks in get_files > > src/security/virt-aa-helper.c | 166 +++--- > tests/virt-aa-helper-test | 15 +++ > 2 files changed, 109 insertions(+), 72 deletions(-) > > -- > 2.24.0 > -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci] guests: Add support for openSUSE Leap 15.1
On Wed, Nov 20, 2019 at 10:46 PM Jim Fehlig wrote: > > On 11/20/19 2:16 PM, Fabiano Fidêncio wrote: > > On Wed, Nov 20, 2019 at 8:44 PM Jim Fehlig wrote: > >> > >> On 11/19/19 11:56 PM, Fabiano Fidêncio wrote: > >>> On Wed, Nov 20, 2019 at 12:01 AM Jim Fehlig wrote: > > On 11/19/19 3:32 AM, Andrea Bolognani wrote: > > On Tue, 2019-11-19 at 00:21 +, Jim Fehlig wrote: > >> > >> [...] > >> > >> +++ b/guests/host_vars/libvirt-opensuse-15.1/main.yml > >> +package_format: 'rpm' > >> +package_manager: 'zypper' > >> +os_name: 'openSUSE' > >> +os_version: '15.1' > > > > So, about the naming. > > > > What I would have done here is > > > > os_name: 'OpenSUSE' > > os_version: '15' > > > > The intial capital letter in os_name goes against the actual branding > > for openSUSE so I'm not perfectly happy with it, but on the other > > hand it's very useful when defining mappings because package formats > > all start with a lowercase letter and all OS names start with an > > uppercase letter. So I would try to stick with that convention. > > Ok, no problem. > > > As for os_version, if you look at all existing entries we use the > > major version number only: eg. we have CentOS7 instead of CentOS7.7 > > and FreeBSD12 instead of FreeBSD12.1: this makes sense because, as > > the guest gets updated over time, it will naturally pick up the > > latest minor release. Will this work for openSUSE too? > > I suppose so. Although for example Leap 15.2 will have a different > kernel (5.3. > vs 4.12), different install path > (http://download.opensuse.org/distribution/leap/15.2/repo/oss/), etc. Is > that okay? > >>> > >>> It depends a lot on what's the OpenSUSE policy of its distro. Once > >>> 15.2 is out, what happens to 15.1? Is this just a "minor" update? Will > >>> 15.1 still be supported or are people expected to just update / > >>> upgrade to 15.2? > >> > >> 15.1 will continue to be actively supported alongside 15.2 for six months > >> after > >> the release of 15.2. The lifecycle is described here > >> > >> https://en.opensuse.org/Lifetime > >> > >> Perhaps it is best to just support the latest Leap release plus openSUSE > >> Tumbleweed? I.e. I send a patch to replace the 15.1 machine with 15.2 once > >> it is > >> released. > > > > I'd adopt the same policy we have for other distros. Support the last > > two releases (as in, Fedora 30 and Fedora 31). > > > > Leap seems to fit in the "Fedora" case. Thus, I'd keep 15.1, 15.2, 15.3, ... > > But how to name those? Is a '.' in the name problematic? Should inventory have > libvirt-opensuse-15.1, libvirt-opensuse-15.2, etc? Now, knowing the release and life cycles, I'd suggest to keep it as: libvirt-opensuse-15.1, libvirt-opensuse-15.2, and so on ... exactly as you did in the patch you submitted. > > > Together with that, I really would like to have Tumbleweed supported as > > well. > > Nod. That one is easy to name: libvirt-opensuse-tw :-). > > Regards, > Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci] guests: Add support for openSUSE Leap 15.1
On 11/20/19 2:16 PM, Fabiano Fidêncio wrote: > On Wed, Nov 20, 2019 at 8:44 PM Jim Fehlig wrote: >> >> On 11/19/19 11:56 PM, Fabiano Fidêncio wrote: >>> On Wed, Nov 20, 2019 at 12:01 AM Jim Fehlig wrote: On 11/19/19 3:32 AM, Andrea Bolognani wrote: > On Tue, 2019-11-19 at 00:21 +, Jim Fehlig wrote: >> >> [...] >> >> +++ b/guests/host_vars/libvirt-opensuse-15.1/main.yml >> +package_format: 'rpm' >> +package_manager: 'zypper' >> +os_name: 'openSUSE' >> +os_version: '15.1' > > So, about the naming. > > What I would have done here is > > os_name: 'OpenSUSE' > os_version: '15' > > The intial capital letter in os_name goes against the actual branding > for openSUSE so I'm not perfectly happy with it, but on the other > hand it's very useful when defining mappings because package formats > all start with a lowercase letter and all OS names start with an > uppercase letter. So I would try to stick with that convention. Ok, no problem. > As for os_version, if you look at all existing entries we use the > major version number only: eg. we have CentOS7 instead of CentOS7.7 > and FreeBSD12 instead of FreeBSD12.1: this makes sense because, as > the guest gets updated over time, it will naturally pick up the > latest minor release. Will this work for openSUSE too? I suppose so. Although for example Leap 15.2 will have a different kernel (5.3. vs 4.12), different install path (http://download.opensuse.org/distribution/leap/15.2/repo/oss/), etc. Is that okay? >>> >>> It depends a lot on what's the OpenSUSE policy of its distro. Once >>> 15.2 is out, what happens to 15.1? Is this just a "minor" update? Will >>> 15.1 still be supported or are people expected to just update / >>> upgrade to 15.2? >> >> 15.1 will continue to be actively supported alongside 15.2 for six months >> after >> the release of 15.2. The lifecycle is described here >> >> https://en.opensuse.org/Lifetime >> >> Perhaps it is best to just support the latest Leap release plus openSUSE >> Tumbleweed? I.e. I send a patch to replace the 15.1 machine with 15.2 once >> it is >> released. > > I'd adopt the same policy we have for other distros. Support the last > two releases (as in, Fedora 30 and Fedora 31). > > Leap seems to fit in the "Fedora" case. Thus, I'd keep 15.1, 15.2, 15.3, ... But how to name those? Is a '.' in the name problematic? Should inventory have libvirt-opensuse-15.1, libvirt-opensuse-15.2, etc? > Together with that, I really would like to have Tumbleweed supported as well. Nod. That one is easy to name: libvirt-opensuse-tw :-). Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci] guests: Add support for openSUSE Leap 15.1
On Wed, Nov 20, 2019 at 8:44 PM Jim Fehlig wrote: > > On 11/19/19 11:56 PM, Fabiano Fidêncio wrote: > > On Wed, Nov 20, 2019 at 12:01 AM Jim Fehlig wrote: > >> > >> On 11/19/19 3:32 AM, Andrea Bolognani wrote: > >>> On Tue, 2019-11-19 at 00:21 +, Jim Fehlig wrote: > > [...] > > +++ b/guests/host_vars/libvirt-opensuse-15.1/main.yml > +package_format: 'rpm' > +package_manager: 'zypper' > +os_name: 'openSUSE' > +os_version: '15.1' > >>> > >>> So, about the naming. > >>> > >>> What I would have done here is > >>> > >>> os_name: 'OpenSUSE' > >>> os_version: '15' > >>> > >>> The intial capital letter in os_name goes against the actual branding > >>> for openSUSE so I'm not perfectly happy with it, but on the other > >>> hand it's very useful when defining mappings because package formats > >>> all start with a lowercase letter and all OS names start with an > >>> uppercase letter. So I would try to stick with that convention. > >> > >> Ok, no problem. > >> > >>> As for os_version, if you look at all existing entries we use the > >>> major version number only: eg. we have CentOS7 instead of CentOS7.7 > >>> and FreeBSD12 instead of FreeBSD12.1: this makes sense because, as > >>> the guest gets updated over time, it will naturally pick up the > >>> latest minor release. Will this work for openSUSE too? > >> > >> I suppose so. Although for example Leap 15.2 will have a different kernel > >> (5.3. > >> vs 4.12), different install path > >> (http://download.opensuse.org/distribution/leap/15.2/repo/oss/), etc. Is > >> that okay? > > > > It depends a lot on what's the OpenSUSE policy of its distro. Once > > 15.2 is out, what happens to 15.1? Is this just a "minor" update? Will > > 15.1 still be supported or are people expected to just update / > > upgrade to 15.2? > > 15.1 will continue to be actively supported alongside 15.2 for six months > after > the release of 15.2. The lifecycle is described here > > https://en.opensuse.org/Lifetime > > Perhaps it is best to just support the latest Leap release plus openSUSE > Tumbleweed? I.e. I send a patch to replace the 15.1 machine with 15.2 once it > is > released. I'd adopt the same policy we have for other distros. Support the last two releases (as in, Fedora 30 and Fedora 31). Leap seems to fit in the "Fedora" case. Thus, I'd keep 15.1, 15.2, 15.3, ... Together with that, I really would like to have Tumbleweed supported as well. Best Regards, -- Fabiano Fidêncio -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci] guests: Add support for openSUSE Leap 15.1
On 11/20/19 1:03 AM, Andrea Bolognani wrote: > On Wed, 2019-11-20 at 07:56 +0100, Fabiano Fidêncio wrote: >> On Wed, Nov 20, 2019 at 12:01 AM Jim Fehlig wrote: >>> On 11/19/19 3:32 AM, Andrea Bolognani wrote: As for os_version, if you look at all existing entries we use the major version number only: eg. we have CentOS7 instead of CentOS7.7 and FreeBSD12 instead of FreeBSD12.1: this makes sense because, as the guest gets updated over time, it will naturally pick up the latest minor release. Will this work for openSUSE too? >>> >>> I suppose so. Although for example Leap 15.2 will have a different kernel >>> (5.3. >>> vs 4.12), different install path >>> (http://download.opensuse.org/distribution/leap/15.2/repo/oss/), etc. Is >>> that okay? >> >> It depends a lot on what's the OpenSUSE policy of its distro. Once >> 15.2 is out, what happens to 15.1? Is this just a "minor" update? Will >> 15.1 still be supported or are people expected to just update / >> upgrade to 15.2? > > Yeah, the changing kernel is not much of an issue (Fedora rawhide, > Debian sid and FreeBSD -CURRENT certainly fits into that category as > well :), it's more a question of whether, as Fabiano said, eg. 15.1 > and 15.2 are considered two separate branches where each of them > continues to get significant development over time, or if (as is the > case for RHEL minor releases) once eg. 7.8 is out, people are mostly > expected to move off 7.7. > > In the latter case, I would expect the transition to be mostly > smooth: running the daily update procedure on a 15.1 machine after > 15.2 has been released would advance the OS to 15.2. That's not > necessarily a hard requirement either, however: for example, FreeBSD > doesn't work like that and requires an explicit upgrade step instead. It sounds like Tumbleweed fits into the later case, whereas Leap is more like FreeBSD. E.g. upgrading from 15.1 to 15.2 requires changing the configured repos (http://download.opensuse.org/distribution/leap/15.2/repo/oss/) and running 'zypper dist-upgrade'. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci] guests: Add support for openSUSE Leap 15.1
On 11/19/19 11:56 PM, Fabiano Fidêncio wrote: > On Wed, Nov 20, 2019 at 12:01 AM Jim Fehlig wrote: >> >> On 11/19/19 3:32 AM, Andrea Bolognani wrote: >>> On Tue, 2019-11-19 at 00:21 +, Jim Fehlig wrote: [...] +++ b/guests/host_vars/libvirt-opensuse-15.1/main.yml +package_format: 'rpm' +package_manager: 'zypper' +os_name: 'openSUSE' +os_version: '15.1' >>> >>> So, about the naming. >>> >>> What I would have done here is >>> >>> os_name: 'OpenSUSE' >>> os_version: '15' >>> >>> The intial capital letter in os_name goes against the actual branding >>> for openSUSE so I'm not perfectly happy with it, but on the other >>> hand it's very useful when defining mappings because package formats >>> all start with a lowercase letter and all OS names start with an >>> uppercase letter. So I would try to stick with that convention. >> >> Ok, no problem. >> >>> As for os_version, if you look at all existing entries we use the >>> major version number only: eg. we have CentOS7 instead of CentOS7.7 >>> and FreeBSD12 instead of FreeBSD12.1: this makes sense because, as >>> the guest gets updated over time, it will naturally pick up the >>> latest minor release. Will this work for openSUSE too? >> >> I suppose so. Although for example Leap 15.2 will have a different kernel >> (5.3. >> vs 4.12), different install path >> (http://download.opensuse.org/distribution/leap/15.2/repo/oss/), etc. Is >> that okay? > > It depends a lot on what's the OpenSUSE policy of its distro. Once > 15.2 is out, what happens to 15.1? Is this just a "minor" update? Will > 15.1 still be supported or are people expected to just update / > upgrade to 15.2? 15.1 will continue to be actively supported alongside 15.2 for six months after the release of 15.2. The lifecycle is described here https://en.opensuse.org/Lifetime Perhaps it is best to just support the latest Leap release plus openSUSE Tumbleweed? I.e. I send a patch to replace the 15.1 machine with 15.2 once it is released. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Speed up syntax-check a little
On 11/20/19 4:34 PM, Daniel Henrique Barboza wrote: On 11/20/19 3:32 PM, Ján Tomko wrote: This shaves off 4 % of syntax-check runtime on my 8-core system, which is enough of a reason for me. Around 10% increase on my system running 'make -j syntax-check'. In time: *performance* increase. Did a few runs, on average these patches made syntax-check 10% faster for me (~11 seconds without, ~10 seconds with them). Thanks, DHB -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Speed up syntax-check a little
On 11/20/19 3:32 PM, Ján Tomko wrote: This shaves off 4 % of syntax-check runtime on my 8-core system, which is enough of a reason for me. Around 10% increase on my system running 'make -j syntax-check'. All patches: Reviewed-by: Daniel Henrique Barboza -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/3] scripts: use in even more
Signed-off-by: Ján Tomko
---
scripts/check-aclrules.py| 2 +-
scripts/check-driverimpls.py | 2 +-
scripts/check-symfile.py | 6 +++---
scripts/dtrace2systemtap.py | 4 ++--
scripts/genpolkit.py | 4 ++--
scripts/gensystemtap.py | 2 +-
6 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/scripts/check-aclrules.py b/scripts/check-aclrules.py
index 40c47c1c6b..1a76ebcb3b 100755
--- a/scripts/check-aclrules.py
+++ b/scripts/check-aclrules.py
@@ -128,7 +128,7 @@ with open(proto, "r") as fh:
api = name_to_ProcName(m.group(1))
# Event filtering is handled in daemon/remote.c
# instead of drivers
-if line.find("_EVENT_REGISTER") == -1:
+if "_EVENT_REGISTER" not in line:
filteredmap[api] = True
incomment = False
diff --git a/scripts/check-driverimpls.py b/scripts/check-driverimpls.py
index ca7ec3af7f..bc3f16a816 100755
--- a/scripts/check-driverimpls.py
+++ b/scripts/check-driverimpls.py
@@ -33,7 +33,7 @@ def checkdriverimpls(filename):
for line in fh:
lineno = lineno + 1
if intable:
-if line.find("}") != -1:
+if "}" in line:
intable = False
mainprefix = None
continue
diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
index a543a0fbcd..0c02591991 100755
--- a/scripts/check-symfile.py
+++ b/scripts/check-symfile.py
@@ -36,9 +36,9 @@ ret = 0
with open(symfile, "r") as fh:
for line in fh:
line = line.strip()
-if line.find("{") != -1:
+if "{" in line:
continue
-if line.find("}") != -1:
+if "}" in line:
continue
if line in ["global:", "local:"]:
continue
@@ -46,7 +46,7 @@ with open(symfile, "r") as fh:
continue
if line[0] == '#':
continue
-if line.find("*") != -1:
+if "*" in line:
continue
line = line.strip(";")
diff --git a/scripts/dtrace2systemtap.py b/scripts/dtrace2systemtap.py
index 922713e599..cf4d8e1a79 100755
--- a/scripts/dtrace2systemtap.py
+++ b/scripts/dtrace2systemtap.py
@@ -49,9 +49,9 @@ with open(dtrace, "r") as fh:
line = line.strip()
if line == "":
continue
-if line.find("provider ") != -1 and line.find("{") != -1:
+if "provider " in line and "{" in line:
continue
-if line.find("};") != -1:
+if "};" in line:
continue
if line.startswith("#"):
diff --git a/scripts/genpolkit.py b/scripts/genpolkit.py
index 0cdba2bd3c..230d920f70 100755
--- a/scripts/genpolkit.py
+++ b/scripts/genpolkit.py
@@ -55,13 +55,13 @@ aclfile = sys.argv[1]
with open(aclfile, "r") as fh:
for line in fh:
if in_opts:
-if line.find("*/") != -1:
+if "*/" in line:
in_opts = False
else:
m = re.search(r'''\*\s*\@(\w+):\s*(.*?)\s*$''', line)
if m is not None:
opts[m.group(1)] = m.group(2)
-elif line.find("**") != -1:
+elif "**" in line:
in_opts = True
else:
m = re.search(r'''VIR_ACCESS_PERM_(%s)_((?:\w|_)+),''' %
diff --git a/scripts/gensystemtap.py b/scripts/gensystemtap.py
index c4bc1620d0..7b391cc911 100755
--- a/scripts/gensystemtap.py
+++ b/scripts/gensystemtap.py
@@ -46,7 +46,7 @@ def load_file(fh):
instatus = True
elif re.search(r'''enum remote_auth_type''', line):
inauth = True
-elif line.find("}") != -1:
+elif "}" in line:
intype = False
instatus = False
inauth = False
--
2.19.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] scripts: speedup prohibit-duplicate-header
Running regular expressions with capture groups is expensive. Bail out early if the line does not start with a '#'. This reduces the runtime of the check by two thirds. Signed-off-by: Ján Tomko --- scripts/prohibit-duplicate-header.py | 4 1 file changed, 4 insertions(+) diff --git a/scripts/prohibit-duplicate-header.py b/scripts/prohibit-duplicate-header.py index dfdfa0bf0b..420311ccef 100644 --- a/scripts/prohibit-duplicate-header.py +++ b/scripts/prohibit-duplicate-header.py @@ -30,6 +30,10 @@ def check_file(filename): for line in fh: lineno = lineno + 1 +# skip non-matching lines early +if line[0] != '#': +continue + headermatch = re.search(r'''^# *include *[<"]([^>"]*\.h)[">]''', line) if headermatch is not None: inc = headermatch.group(1) -- 2.19.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] scripts: check-aclrules: use regular expressions less often
Use a simple
if "substr" in line
before running a regular expression, which saves time,
especially if the regex has a capture group.
This reduces runtime of the check by almost 78 % for me.
Signed-off-by: Ján Tomko
---
scripts/check-aclrules.py | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/scripts/check-aclrules.py b/scripts/check-aclrules.py
index d145e59164..40c47c1c6b 100755
--- a/scripts/check-aclrules.py
+++ b/scripts/check-aclrules.py
@@ -150,13 +150,21 @@ def process_file(filename):
# Looks for anything which appears to be a function
# body name. Doesn't matter if we pick up bogus stuff
# here, as long as we don't miss valid stuff
-m = re.search(r'''\b(\w+)\(''', line)
+m = None
+if "(" in line:
+m = re.search(r'''\b(\w+)\(''', line)
if m is not None:
maybefunc = m.group(1)
elif brace > 0:
-ensureacl = re.search(r'''(\w+)EnsureACL''', line)
-checkacl = re.search(r'''(\w+)CheckACL''', line)
-stub = re.search(r'''\b(\w+)\(''', line)
+ensureacl = None
+checkacl = None
+stub = None
+if "EnsureACL" in line:
+ensureacl = re.search(r'''(\w+)EnsureACL''', line)
+if "CheckACL" in line:
+checkacl = re.search(r'''(\w+)CheckACL''', line)
+if "(" in line:
+stub = re.search(r'''\b(\w+)\(''', line)
if ensureacl is not None:
# Record the fact that maybefunc contains an
# ACL call, and make sure it is the right call!
@@ -210,7 +218,9 @@ def process_file(filename):
# every func listed there, has an impl which calls
# an ACL function
if intable:
-assign = re.search(r'''\.(\w+)\s*=\s*(\w+),?''', line)
+assign = None
+if '"' in line:
+assign = re.search(r'''\.(\w+)\s*=\s*(\w+),?''', line)
if "}" in line:
intable = False
table = None
@@ -237,8 +247,10 @@ def process_file(filename):
file=sys.stderr)
errs = True
else:
-m = re.search(r'''^(?:static\s+)?(vir(?:\w+)?Driver)\s+''',
- line)
+m = None
+if "Driver" in line:
+m = re.search(r'''^(?:static\s+)?(vir(?:\w+)?Driver)\s+''',
+ line)
if m is not None:
name = m.group(1)
if name not in ["virNWFilterCallbackDriver",
--
2.19.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] scripts: check-aclrules: use in instead of find
For checking whether a substring is present in a string,
using the pattern:
"str" in string
is slightly faster than:
string.find("str") != -1
Use it to shave off 4 % of the runtime of this script that
processes quite a few long source files.
Signed-off-by: Ján Tomko
---
scripts/check-aclrules.py | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/scripts/check-aclrules.py b/scripts/check-aclrules.py
index e06a019b00..d145e59164 100755
--- a/scripts/check-aclrules.py
+++ b/scripts/check-aclrules.py
@@ -97,7 +97,7 @@ def fixup_name(name):
def name_to_ProcName(name):
elems = []
-if name.find("_") != -1 or name.lower() in ["open", "close"]:
+if "_" in name or name.lower() in ["open", "close"]:
elems = [n.lower().capitalize() for n in name.split("_")]
else:
elems = [name]
@@ -116,11 +116,11 @@ with open(proto, "r") as fh:
filtered = False
for line in fh:
-if line.find("/**") != -1:
+if "/**" in line:
incomment = True
filtered = False
elif incomment:
-if line.find("* @aclfilter") != -1:
+if "* @aclfilter" in line:
filtered = True
elif filtered:
m = re.search(r'''REMOTE_PROC_(.*)\s+=\s*\d+''', line)
@@ -211,7 +211,7 @@ def process_file(filename):
# an ACL function
if intable:
assign = re.search(r'''\.(\w+)\s*=\s*(\w+),?''', line)
-if line.find("}") != -1:
+if "}" in line:
intable = False
table = None
elif assign is not None:
@@ -247,9 +247,9 @@ def process_file(filename):
intable = True
table = name
-if line.find("{") != -1:
+if "{" in line:
brace = brace + 1
-if line.find("}") != -1:
+if "}" in line:
brace = brace - 1
return errs
--
2.19.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Speed up syntax-check a little
This shaves off 4 % of syntax-check runtime on my 8-core system, which is enough of a reason for me. Surely it saves more on our overloaded CI workers. Ján Tomko (3): scripts: speedup prohibit-duplicate-header scripts: check-aclrules: use in instead of find scripts: check-aclrules: use regular expressions less often scripts/check-aclrules.py| 38 ++-- scripts/prohibit-duplicate-header.py | 4 +++ 2 files changed, 29 insertions(+), 13 deletions(-) -- 2.19.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/8] domcaps: Add function for initializing domain caps as unsupported
On 11/20/19 9:40 AM, Peter Krempa wrote:
> On Mon, Nov 18, 2019 at 14:43:08 -0500, Cole Robinson wrote:
>> On 11/13/19 11:05 AM, Peter Krempa wrote:
>>> 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;
>>> +}
>>> +
>>> +
>>
>> This pattern is suboptimal IMO. Any time a new domcaps capability is
>> added, say to serve the qemu/ driver, the developer now needs to
>> consider how this shared function impacts libxl domcaps XML.
>
> I see your point, but I think there's merit also towards the other
> approach.
>
> Here we explicitly mark everything as unsupported, but that does not
> prevent developers from resetting the caps to the missing state in case
> when it's ambiguous or they don't wish to deal with other hypervisors.
>
>>
>> Say hypothetically tomorrow I want to expose 'acpi' support in domcaps
>> . My main goal is to expose this in qemu domcaps. Naturally a
>> starting point is to add 'caps->acpi = VIR_TRISTATE_BOOL_NO;' here.
>>
>> If I don't remember to check libxl code though, I've now potentially
>> converted their driver to report blatantly incorrect domcaps output, as
>> libxl does support ACPI. If I do remember to check their code, it's my
>> responsibility to edit libxl code to ensure it's reporting accurate
>> information, which makes my life harder.
>
> In my opinion it's not wrong to ask developers to at least think whether
> the change does not influence other hypervisors as well or in case when
> it's trivially supported to more than the original intention.
>
I agree with that statement. Devs should be considering other drivers
when extending domcaps schema. That doesn't change with the current way
or the previous way. What changes is the default result if other drivers
are not considered properly:
* old way:
** libxl/bhyve driver is ignored: their XML output doesn't change at
all. missed opportunity to improve their XML output coverage, but
otherwise the status quo is maintained
** libxl/bhyve driver is considered: requires explicit changes in their
code to support supported='no' or supported='yes' case.
* git way:
** libxl/bhyve driver is ignored.
*** 1) supported='no' is correct state. only likely result is CI
breakage or review issues with bhyve caps needing regenerating
*** 2) supported='no' is incorrect state. unless review catches this,
libxl/bhyve now may be reporting blatantly incorrect info.
** libxl/bhyve driver is considered: supported='no' requires no code
change, but supported='yes' requires some code change
>From my perspective as an app developer, here's my order of worst to
best cases for domcaps output:
1) domcaps output is present but incorrect
2) domcaps output is absent
3) domcaps output is present and correct
And there's a giant distance between 1 and 2. #2 is basically the
default domcaps state for a whole bunch of interesting information, apps
already can't depend on it existing because it's not provided by any driver.
The case we really should be optimizing for IMO is avoiding #1, and the
current git code makes it easier to introduce errors like that.
I admit, it's kind of a philosophical argument for the XML. I
expect most things that features are extended for will only count for
the qemu driver. But if this pattern were to be extended to things like
Re: [libvirt] [PATCH v2 3/3] virt-aa-helper: drop pointer checks in get_files
On 11/20/19 12:25 PM, Jamie Strandboge wrote: > On Wed, 20 Nov 2019, Cole Robinson wrote: > >> On 11/19/19 4:31 PM, Jamie Strandboge wrote: >>> On Thu, 14 Nov 2019, Christian Ehrhardt wrote: >>> It was mentioned that the pointers in loops like: for (i = 0; i < ctl->def->nserials; i++) if (ctl->def->serials[i] ... will always be !NULL or we would have a much more serious problem. Simplify the if chains in get_files by dropping these checks. >>> >>> I don't really see why a NULL check is a problem so this feels a bit >>> like busy work and it seems short-circuiting and not doing is exactly >>> what you would want to do in the event of a 'much more serious problem'. >>> Is this really required? +0 to apply on principle (I've not reviewed the >>> changes closely). >>> >> >> If for example def->nserials and def->serials[i] disagree, then there is >> a serious bug somewhere. The internal API contract is that it should >> never happen, so code shouldn't check for the condition. I'm pretty sure >> if the XML parser is creating that situation, the qemu driver would >> crash in a dozen different places. >> >> So the patch isn't strictly required, but it is an improvement IMO: it >> reduces code, improves readability, and helps simplify review for other >> libvirt devs who may be confused by this uncommon idiom (I was). But I >> will leave it up to you guys to decide whether to push or not > > To be clear, I am intentionally not blocking on this. If, as upstream, > you'd prefer this to be a certain way for reasons that outweigh my POV, > by all means feel free to do that. The changes are mechanical and IMHO > don't need an apparmor-focused review (though if you'd prefer I go > through the full patch, I can). > This patch moves the code to be more consistent with the rest of the libvirt code base, so I think it's good to push. I thought Christian was waiting on you for review before pushing apparmor patches though, I didn't want to step on any toes :) Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] virt-aa-helper: drop pointer checks in get_files
On Wed, 20 Nov 2019, Cole Robinson wrote: > On 11/19/19 4:31 PM, Jamie Strandboge wrote: > > On Thu, 14 Nov 2019, Christian Ehrhardt wrote: > > > >> It was mentioned that the pointers in loops like: > >> for (i = 0; i < ctl->def->nserials; i++) > >> if (ctl->def->serials[i] ... > >> will always be !NULL or we would have a much more serious problem. > >> > >> Simplify the if chains in get_files by dropping these checks. > > > > I don't really see why a NULL check is a problem so this feels a bit > > like busy work and it seems short-circuiting and not doing is exactly > > what you would want to do in the event of a 'much more serious problem'. > > Is this really required? +0 to apply on principle (I've not reviewed the > > changes closely). > > > > If for example def->nserials and def->serials[i] disagree, then there is > a serious bug somewhere. The internal API contract is that it should > never happen, so code shouldn't check for the condition. I'm pretty sure > if the XML parser is creating that situation, the qemu driver would > crash in a dozen different places. > > So the patch isn't strictly required, but it is an improvement IMO: it > reduces code, improves readability, and helps simplify review for other > libvirt devs who may be confused by this uncommon idiom (I was). But I > will leave it up to you guys to decide whether to push or not To be clear, I am intentionally not blocking on this. If, as upstream, you'd prefer this to be a certain way for reasons that outweigh my POV, by all means feel free to do that. The changes are mechanical and IMHO don't need an apparmor-focused review (though if you'd prefer I go through the full patch, I can). -- Jamie Strandboge | http://www.canonical.com signature.asc Description: PGP signature -- 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 Wed, 20 Nov 2019, Christian Ehrhardt wrote:
> On Tue, Nov 19, 2019 at 10:25 PM Jamie Strandboge wrote:
> > On Tue, 22 Oct 2019, Christian Ehrhardt wrote:
> > > +for (i = 0; i < ctl->def->nshmems; i++) {
> > > +if (ctl->def->shmems[i]) {
> > > +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;
> >
> > I'll let others comment on the code changes, but this apparmor rule
> > looks ok.
> >
> > > +} else {
> >
> > That said, I wonder about the logic here since up above we have:
> >
> > if (shmem->server.enabled && shmem->server.chr.data.nix.path)
> >
> > but here we just have 'else'. What happens if server.enabled is false
> > and server.chr.data.nix.path is set or vice versa? Does this 'else'
> > clause correctly handle those scenarios?
>
> Yes if either of the above isn't fulfilled it will fall back to use
> the default paths.
> So a single else without other checks should be correct.
> The following switch then differs the default paths used base on the model.
Thanks! We agreed on irc a small code comment would clarify this. LGTM
provided you add a code comment similar to what we discussed on IRC.
--
Jamie Strandboge | http://www.canonical.com
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] apparmor: let AppArmorSetSecurityImageLabel append rules
On Wed, 20 Nov 2019, Christian Ehrhardt wrote: > On Wed, Nov 20, 2019 at 3:40 PM Christian Ehrhardt > wrote: > > > > On Tue, Nov 19, 2019 at 10:12 PM Jamie Strandboge > > wrote: > > > > > > On Wed, 16 Oct 2019, Christian Ehrhardt wrote: > > > > > > > There are currently broken use cases, e.g. snapshotting more than one > > > > disk at > > > > once like: > > > > $ virsh snapshot-create-as --domain eoan --disk-only --atomic > > > >--diskspec vda,snapshot=no --diskspec vdb,snapshot=no > > > >--diskspec vdc,file=/test/disk1.snapshot1.qcow,snapshot=external > > > >--diskspec vdd,file=/test/disk2.snapshot1.qcow,snapshot=external > > > > The command above will iterate from qemuDomainSnapshotCreateDiskActive > > > > and > > > > eventually add /test/disk1.snapshot1.qcow first (appears in the rules) > > > > to then later add /test/disk2.snapshot1.qcow and while doing so throwing > > > > away the former rule causing it to fail. > > > > > > > > All other calls to (re)load_profile already use append=true when adding > > > > rules append=false is only used when restoring rules [1]. > > > > > > > > Fix this by letting AppArmorSetSecurityImageLabel use append=true as > > > > well. > > > > > > > > Bugs: > > > > https://bugs.launchpad.net/libvirt/+bug/1845506 > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1746684 > > > > > > > > [1]: https://bugs.launchpad.net/libvirt/+bug/1845506/comments/13 > > > > > > > > Signed-off-by: Christian Ehrhardt > > > > --- > > > > src/security/security_apparmor.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/src/security/security_apparmor.c > > > > b/src/security/security_apparmor.c > > > > index 320d69e52a..4dd7ba20b4 100644 > > > > --- a/src/security/security_apparmor.c > > > > +++ b/src/security/security_apparmor.c > > > > @@ -812,7 +812,7 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr > > > > mgr, > > > > return -1; > > > > } > > > > > > > > -return reload_profile(mgr, def, src->path, false); > > > > +return reload_profile(mgr, def, src->path, true); > > > > > > src/security/security_manager.c shows that > > > virSecurityManagerSetImageLabel() is called to label 'a storage image > > > with the configured security label'. Based on your report, it seems that > > > in addition to the underlying disk (vda in this case), it is also called > > > for each additional disk (ie, vdc and vdd). > > > > Yes in the "transaction" for e.g. a snapshot on multiple disks there will be > > calls for both disks before any of them becomes part of the VM-definition. > > That is where the second call "removes" the first rule and then things fail. > > > > To be clear, I didn't add any "disk" in the broken use-case it adds > > further backing image chain elements to existing disks as part of the > > snapshot action. > > So if I snapshot in one shot vdb and vdc it is called for each of > > those adding the new paths for the respective backing chain that > > grows. > > > > If you ever power cycle that it will be part of the definition and > > virt-aa-helper resolves backing chains as needed. > > > > > While your patch to append > > > makes sense for this scenario, what happens if you update the vm > > > definition to use 'vde' instead of 'vda', is the vda access removed and > > > vde added with vdc and vdd remaining? > > > > Hmm - not sure what exactly you mean, but lets try to break it ... > > > > This splits into three major paths to consider: > > > > a) adding/removing disks while the guest is off. > > This isn't interesting as that way it will be part (or not) of the > > definition when it starts. > > The guest will get initial rules based on that definition on > > start, nothing special. > > > > b) adding/removing disks at runtime of a guest > >b1) you can edit a live definition in XML, but that will only > > add/modify the disk on next start > > Even if you now add another disk via proper hot-add later the > > first will not be added > > as it isn't really part of the active definition yet. > >b2) you can hot add/remove a disk, those will be properly labelled > > and added/removed > > via their labelling calls on that action > > > > c) snapshots at runtime of the guest (that was the broken case) > > As mentioned above it will need to add new elements to the > > backing-chain > > c1) snapshot one disk will work, the call to > > AppArmorSetSecurityImageLabel will add the new rule needed > > c2) snapshot multiple disks at once then it will fail without the fix > > here > > the second call to AppArmorSetSecurityImageLabel will revoke > > the former rule > > > > Sorry, I don't find the " update the vm definition to use 'vde' > > instead of 'vda'" that you meant in either of the paths a/b/c above. > > I'll try to reach you on IRC later on to sort this out ... > > After discussing on IRC (thanks) and understanding that it wasn't so > much about the
Re: [libvirt] [PATCH v2 1/4] qemu: block: propagate the delete flag to where it can actually be used
On Wed, Nov 20, 2019 at 12:14:11PM +0100, Peter Krempa wrote:
> On Wed, Nov 20, 2019 at 11:44:52 +0100, Pavel Mores wrote:
> > Since the blockcommit operation is asynchronous, this has conceptually two
> > parts. First, we have to propagate the flag from qemuDomainBlockCommit()
> > (which was just ignoring it until now) to qemuBlockJobDiskNewCommit(). Then
> > it can be stored in the qemuBlockJobCommitData structure which holds
> > information necessary to finish the job asynchronously.
> >
> > Signed-off-by: Pavel Mores
> > ---
> > src/qemu/qemu_blockjob.c | 4 +++-
> > src/qemu/qemu_blockjob.h | 4 +++-
> > src/qemu/qemu_driver.c | 5 +++--
> > 3 files changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> > index 5c294f8024..7d94a6ce38 100644
> > --- a/src/qemu/qemu_blockjob.c
> > +++ b/src/qemu/qemu_blockjob.c
> > @@ -250,7 +250,8 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
> >virDomainDiskDefPtr disk,
> >virStorageSourcePtr topparent,
> >virStorageSourcePtr top,
> > - virStorageSourcePtr base)
> > + virStorageSourcePtr base,
> > + bool delete_imgs)
> > {
> > qemuDomainObjPrivatePtr priv = vm->privateData;
> > g_autoptr(qemuBlockJobData) job = NULL;
> > @@ -273,6 +274,7 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
> > job->data.commit.topparent = topparent;
> > job->data.commit.top = top;
> > job->data.commit.base = base;
> > +job->data.commit.deleteCommittedImages = delete_imgs;
> >
> > if (qemuBlockJobRegister(job, vm, disk, true) < 0)
> > return NULL;
> > diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
> > index d8da918f2f..d77f1dcdb3 100644
> > --- a/src/qemu/qemu_blockjob.h
> > +++ b/src/qemu/qemu_blockjob.h
> > @@ -85,6 +85,7 @@ struct _qemuBlockJobCommitData {
> > virStorageSourcePtr topparent;
> > virStorageSourcePtr top;
> > virStorageSourcePtr base;
> > +bool deleteCommittedImages;
> > };
> >
> >
> > @@ -165,7 +166,8 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
> >virDomainDiskDefPtr disk,
> >virStorageSourcePtr topparent,
> >virStorageSourcePtr top,
> > - virStorageSourcePtr base);
> > + virStorageSourcePtr base,
> > + bool delete_imgs);
> >
> > qemuBlockJobDataPtr
> > qemuBlockJobNewCreate(virDomainObjPtr vm,
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index dc14ec86a3..75458f5c8a 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -18496,10 +18496,10 @@ qemuDomainBlockCommit(virDomainPtr dom,
> > bool persistjob = false;
> > bool blockdev = false;
> >
> > -/* XXX Add support for COMMIT_DELETE */
> > virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
> >VIR_DOMAIN_BLOCK_COMMIT_ACTIVE |
> >VIR_DOMAIN_BLOCK_COMMIT_RELATIVE |
> > + VIR_DOMAIN_BLOCK_COMMIT_DELETE |
> >VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES, -1);
> >
> > if (!(vm = qemuDomainObjFromDomain(dom)))
> > @@ -18638,7 +18638,8 @@ qemuDomainBlockCommit(virDomainPtr dom,
> > goto endjob;
> >
> > if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSource,
> > - baseSource)))
> > + baseSource,
> > + flags &
> > VIR_DOMAIN_BLOCK_COMMIT_DELETE)))
> > goto endjob;
>
> I'd prefer if these last two hunks which enable the feature are in a
> separate commit at the end of the series, so that they enable it only
> once all the plumbing is in place.
I tried to figure out how to do this but I'm afraid I don't see a very good
way. Here's my thinking: since code in patch 2 reads and uses
qemuBlockJobCommitData.deleteCommittedImages, patch 1 should make sure it's
there and it's initialised. So patch 1 should add deleteCommittedImages to
qemuBlockJobCommitData and qemuBlockJobDiskNewCommit() should set it to a
value. To get that value, it needs to take the extra argument. However, if
it does take the extra argument, qemuDomainBlockCommit() has to pass it.
So (without resorting to an initialisation to a dummy value), I can't really
see a natural way how to split this patch. Also, the actual enabling of the
feature doesn't happen anyway until patch 2 where deleteCommittedImages is
read and acted upon - until then, patch 1 - even as it is now - is just
setting a value that no-one reads. I'm not sure if it's worth the trouble -
am I overlooking something?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/7] docs: fix ability to view web pages from build tree
On Wed, Nov 20, 2019 at 03:28:47PM +, Daniel P. Berrangé wrote: > Some of the web content is only present in the source tree, thus when > viewing pages from the build tree they appear missing. > > Signed-off-by: Daniel P. Berrangé > --- > docs/Makefile.am | 24 > 1 file changed, 24 insertions(+) > > diff --git a/docs/Makefile.am b/docs/Makefile.am > index 50b2fd7066..0c828102e1 100644 > --- a/docs/Makefile.am > +++ b/docs/Makefile.am > @@ -39,6 +39,30 @@ modules_admin = libvirt-admin > modules_qemu = libvirt-qemu > modules_lxc = libvirt-lxc > > +all: vpathhack > + > +# This hack enables us to view the web pages > +# from within the uninstalled build tree > +vpathhack: > + for dir in fonts js logos; \ This should be @for dir in fonts js logos; \ to make it quiet > + do \ > + ln -s $(srcdir)/$$dir $$dir ; \ This should be prefixed by 'test -e $$dir || ' > + done > + for file in $(css); \ likewise > + do \ > + ln -s $(srcdir)/$$file $$file ; \ This should be prefixed by 'test -e $$file || ' > + done I've made this change locally but won't post a v2 unless there are other changes needing a respin too. 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/1] virt-aa-helper: Add support for smartcard host-certificates
Jamie Strandboge writes:
> On Thu, 14 Nov 2019, Cole Robinson wrote:
>
>> On 10/24/19 4:57 AM, Arnaud Patard wrote:
>> > When emulating smartcard with host certificates, qemu needs to
>> > be able to read the certificates files, which is denied by apparmor.
>> > Add necessary code to add the smartcard certificates related directory
>> > to the apparmor profile.
>> >
>> > This code supports only this case smartcard 'host' and 'passthrough'
>> > settings are not supported, as I can't test them.
>> >
>> > Signed-off-by: Arnaud Patard
>> > Index: libvirt-5.0.0/src/security/virt-aa-helper.c
>> > ===
>> > --- libvirt-5.0.0.orig/src/security/virt-aa-helper.c
>> > +++ libvirt-5.0.0/src/security/virt-aa-helper.c
>> > @@ -1251,6 +1251,26 @@ get_files(vahControl * ctl)
>> > }
>> > }
>> >
>> > +for (i = 0; i < ctl->def->nsmartcards; i++) {
>> > +virDomainSmartcardDefPtr sc = ctl->def->smartcards[i];
>> > +virDomainSmartcardType sc_type = sc->type;
>> > +char *sc_db = (char *)VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE;
>> > +if (sc->data.cert.database)
>> > +sc_db = sc->data.cert.database;
>> > +switch(sc_type) {
>>
>> Add a space after 'switch'. 'make syntax-check' will catch this. libvirt
>> style is typically to not indent the 'case' keyword either, but this
>> file is inconsistent on that front. With those fixed:
>>
>> Reviewed-by: Cole Robinson
>>
>> This matches what is done for the selinux driver AFAICT
>>
>> CCing apparmor maintainers, I'll defer to them to commit
>>
>> - Cole
>>
>> > +case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
>> > +break;
>> > +case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
>> > +virBufferAsprintf(, " \"%s/\" rk,\n", sc_db);
>> > +virBufferAsprintf(, " \"%s/*\" rk,\n", sc_db);
>> > +break;
>
> I'll let other comment on the code changes, but these rules look ok. I
> might suggest using this one line instead:
>
> virBufferAsprintf(, " \"%s/{,*}" rk,\n", sc_db);
>
Ok, will update my patch to use this.
> Is it possible that the certificates might be in a lower directory? Ie,
> is '**' warranted?
>
In the case of host certificates, the database parameter corresponds to
a NSS db directory, for instance:
$ ls /
cert9.db key4.db pkcs11.txt
The certificate are in the db files and the name given in the xml are
the names of the certificates in the db:
$ certutil -L -d sql:
Certificate Nickname Trust
Attributes
SSL,S/MIME,JAR/XPI
cert1CTu,Cu,Cu
cert2CTu,Cu,Cu
cert3CTu,Cu,Cu
This means only the database directory is interesting.
>> > +case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
>> > +break;
>> > +case VIR_DOMAIN_SMARTCARD_TYPE_LAST:
>> > +break;
>> > +}
>> > +}
>
> It probably makes sense to add a code comment on why
> VIR_DOMAIN_SMARTCARD_TYPE_HOST, VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH
> and VIR_DOMAIN_SMARTCARD_TYPE_LAST aren't supported and ignored.
ok, will add a comment about not being able to test theses cases.
Arnaud.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] virt-aa-helper: drop pointer checks in get_files
On 11/19/19 4:31 PM, Jamie Strandboge wrote: > On Thu, 14 Nov 2019, Christian Ehrhardt wrote: > >> It was mentioned that the pointers in loops like: >> for (i = 0; i < ctl->def->nserials; i++) >> if (ctl->def->serials[i] ... >> will always be !NULL or we would have a much more serious problem. >> >> Simplify the if chains in get_files by dropping these checks. > > I don't really see why a NULL check is a problem so this feels a bit > like busy work and it seems short-circuiting and not doing is exactly > what you would want to do in the event of a 'much more serious problem'. > Is this really required? +0 to apply on principle (I've not reviewed the > changes closely). > If for example def->nserials and def->serials[i] disagree, then there is a serious bug somewhere. The internal API contract is that it should never happen, so code shouldn't check for the condition. I'm pretty sure if the XML parser is creating that situation, the qemu driver would crash in a dozen different places. So the patch isn't strictly required, but it is an improvement IMO: it reduces code, improves readability, and helps simplify review for other libvirt devs who may be confused by this uncommon idiom (I was). But I will leave it up to you guys to decide whether to push or not Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: limit completion of 'domhostname' to active domains
On 11/20/19 2:35 PM, Pino Toscano wrote: Getting the hostname of guest usually requires a in-guest agent, or generally can be determined only on active domains. Signed-off-by: Pino Toscano --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Michal Privoznik Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-users] FYI mail problems for libvirt lists
On Tue, Nov 19, 2019 at 10:00:05AM +, Daniel P. Berrangé wrote: > It has come to our attention that many, possibly even all, people with > non-redhat.com email addresses are unable to send mail to most libvirt > mailing lists, receiving bounce messages saying the address doesn't > exist eg > > Final-Recipient: rfc822; libvirt-us...@redhat.com > Action: failed > Status: 5.0.0 > Remote-MTA: dns; us-smtp-inbound-1.mimecast.com > Diagnostic-Code: smtp; 550 Invalid Recipient - > https://community.mimecast.com/docs/DOC-1369#550 > [boadZMN8PrGodBBK6pzKWg.us309] > > In my testing, libvir-list@redhat.com is the only list that is currently > accepting incoming mail from non-redhat.com addresses. > > I see bounces from libvirt-us...@redhat.com, libvirt...@redhat.com, > libvirt-annou...@redhat.com and libvirt-secur...@redhat.com, as well > as from the undocumented alias libvirt-l...@redhat.com I believe this issue is now resolved and the lists are accepting incoming mail normally. 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 4/4] apparmor: let AppArmorSetSecurityImageLabel append rules
On Wed, Nov 20, 2019 at 3:40 PM Christian Ehrhardt wrote: > > On Tue, Nov 19, 2019 at 10:12 PM Jamie Strandboge wrote: > > > > On Wed, 16 Oct 2019, Christian Ehrhardt wrote: > > > > > There are currently broken use cases, e.g. snapshotting more than one > > > disk at > > > once like: > > > $ virsh snapshot-create-as --domain eoan --disk-only --atomic > > >--diskspec vda,snapshot=no --diskspec vdb,snapshot=no > > >--diskspec vdc,file=/test/disk1.snapshot1.qcow,snapshot=external > > >--diskspec vdd,file=/test/disk2.snapshot1.qcow,snapshot=external > > > The command above will iterate from qemuDomainSnapshotCreateDiskActive and > > > eventually add /test/disk1.snapshot1.qcow first (appears in the rules) > > > to then later add /test/disk2.snapshot1.qcow and while doing so throwing > > > away the former rule causing it to fail. > > > > > > All other calls to (re)load_profile already use append=true when adding > > > rules append=false is only used when restoring rules [1]. > > > > > > Fix this by letting AppArmorSetSecurityImageLabel use append=true as well. > > > > > > Bugs: > > > https://bugs.launchpad.net/libvirt/+bug/1845506 > > > https://bugzilla.redhat.com/show_bug.cgi?id=1746684 > > > > > > [1]: https://bugs.launchpad.net/libvirt/+bug/1845506/comments/13 > > > > > > Signed-off-by: Christian Ehrhardt > > > --- > > > src/security/security_apparmor.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/src/security/security_apparmor.c > > > b/src/security/security_apparmor.c > > > index 320d69e52a..4dd7ba20b4 100644 > > > --- a/src/security/security_apparmor.c > > > +++ b/src/security/security_apparmor.c > > > @@ -812,7 +812,7 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr > > > mgr, > > > return -1; > > > } > > > > > > -return reload_profile(mgr, def, src->path, false); > > > +return reload_profile(mgr, def, src->path, true); > > > > src/security/security_manager.c shows that > > virSecurityManagerSetImageLabel() is called to label 'a storage image > > with the configured security label'. Based on your report, it seems that > > in addition to the underlying disk (vda in this case), it is also called > > for each additional disk (ie, vdc and vdd). > > Yes in the "transaction" for e.g. a snapshot on multiple disks there will be > calls for both disks before any of them becomes part of the VM-definition. > That is where the second call "removes" the first rule and then things fail. > > To be clear, I didn't add any "disk" in the broken use-case it adds > further backing image chain elements to existing disks as part of the > snapshot action. > So if I snapshot in one shot vdb and vdc it is called for each of > those adding the new paths for the respective backing chain that > grows. > > If you ever power cycle that it will be part of the definition and > virt-aa-helper resolves backing chains as needed. > > > While your patch to append > > makes sense for this scenario, what happens if you update the vm > > definition to use 'vde' instead of 'vda', is the vda access removed and > > vde added with vdc and vdd remaining? > > Hmm - not sure what exactly you mean, but lets try to break it ... > > This splits into three major paths to consider: > > a) adding/removing disks while the guest is off. > This isn't interesting as that way it will be part (or not) of the > definition when it starts. > The guest will get initial rules based on that definition on > start, nothing special. > > b) adding/removing disks at runtime of a guest >b1) you can edit a live definition in XML, but that will only > add/modify the disk on next start > Even if you now add another disk via proper hot-add later the > first will not be added > as it isn't really part of the active definition yet. >b2) you can hot add/remove a disk, those will be properly labelled > and added/removed > via their labelling calls on that action > > c) snapshots at runtime of the guest (that was the broken case) > As mentioned above it will need to add new elements to the backing-chain > c1) snapshot one disk will work, the call to > AppArmorSetSecurityImageLabel will add the new rule needed > c2) snapshot multiple disks at once then it will fail without the fix here > the second call to AppArmorSetSecurityImageLabel will revoke > the former rule > > Sorry, I don't find the " update the vm definition to use 'vde' > instead of 'vda'" that you meant in either of the paths a/b/c above. > I'll try to reach you on IRC later on to sort this out ... After discussing on IRC (thanks) and understanding that it wasn't so much about the new change in this patch but a general "are rules revoked as expected" I came up with this tests: Setup: vca (root) vdb (helper disk not used here) vdc /var/lib/uvtool/libvirt/images/focal-test2.qcow (present at guest start) vdd /var/lib/uvtool/libvirt/images/focal-test1.qcow (hot-added after
[libvirt] [PATCH 7/7] docs: fix include of ACL permissions files
The XSL generator loads included HTML files relative to the source dir
but we need to tell it to load them from the build dir instead.
Signed-off-by: Daniel P. Berrangé
---
docs/Makefile.am | 1 +
docs/newapi.xsl | 1 -
docs/page.xsl| 4 +++-
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/docs/Makefile.am b/docs/Makefile.am
index 0c828102e1..d4633b5d2f 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -293,6 +293,7 @@ EXTRA_DIST += \
style=subsite.xsl; \
fi; \
$(XSLTPROC) --stringparam pagename $$name \
+ --stringparam builddir '$(abs_top_builddir)' \
--stringparam timestamp $(timestamp) --nonet \
$(top_srcdir)/docs/$$style $< > $@ \
|| { rm $@ && exit 1; }
diff --git a/docs/newapi.xsl b/docs/newapi.xsl
index dd6169397b..670879dc48 100644
--- a/docs/newapi.xsl
+++ b/docs/newapi.xsl
@@ -22,7 +22,6 @@
-
diff --git a/docs/page.xsl b/docs/page.xsl
index 65ddeb2bb7..6f429ae087 100644
--- a/docs/page.xsl
+++ b/docs/page.xsl
@@ -7,6 +7,8 @@
exclude-result-prefixes="xsl exsl html"
version="1.0">
+
+
@@ -168,7 +170,7 @@
-
+
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/7] docs: remove unused make targets
Signed-off-by: Daniel P. Berrangé --- docs/Makefile.am | 5 - 1 file changed, 5 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index db9470ae07..50b2fd7066 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -232,11 +232,6 @@ timestamp="$(shell if test -n "$$SOURCE_DATE_EPOCH"; \ date -u; \ fi)" -api: libvirt-api.xml -qemu_api: libvirt-qemu-api.xml -lxc_api: libvirt-lxc-api.xml -admin_api: libvirt-admin-api.xml - hvsupport.html: hvsupport.html.in hvsupport.html.in: $(srcdir)/hvsupport.pl $(api_DATA) \ -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/7] docs: use variable for referencing API XML filenames
Signed-off-by: Daniel P. Berrangé --- docs/Makefile.am | 22 ++ 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index 5f59d057f1..a360b546d6 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -146,18 +146,6 @@ dot_html = \ htmldir = $(HTML_DIR) html_DATA = $(css) $(png) $(gif) $(dot_html) -xml = \ - libvirt-api.xml - -qemu_xml = \ - libvirt-qemu-api.xml - -lxc_xml = \ - libvirt-lxc-api.xml - -admin_xml = \ - libvirt-admin-api.xml - apidir = $(pkgdatadir)/api api_DATA = \ libvirt-api.xml \ @@ -202,10 +190,7 @@ CLEANFILES = \ $(apihtml) \ $(internals_html) \ $(kbase_html) \ - $(xml) \ - $(qemu_xml) \ - $(lxc_xml) \ - $(admin_xml) \ + $(api_DATA) \ $(dot_html_generated_in) \ aclperms.htmlinc @@ -280,10 +265,7 @@ python_generated_files = \ html/libvirt-libvirt-qemu.html \ html/libvirt-libvirt-admin.html \ html/libvirt-virterror.html \ - libvirt-api.xml \ - libvirt-lxc-api.xml \ - libvirt-qemu-api.xml \ - libvirt-admin-api.xml \ + $(api_DATA) \ $(NULL) APIBUILD=$(srcdir)/apibuild.py -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/7] docs: generate API reference pages for admin, qemu & lxc libraries
Signed-off-by: Daniel P. Berrangé
---
docs/Makefile.am | 44 +++-
docs/docs.html.in | 4
docs/newapi.xsl | 13 +++--
3 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/docs/Makefile.am b/docs/Makefile.am
index a360b546d6..db9470ae07 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -35,6 +35,10 @@ modules = \
virterror \
$(NULL)
+modules_admin = libvirt-admin
+modules_qemu = libvirt-qemu
+modules_lxc = libvirt-lxc
+
apihtml = \
html/index.html \
$(apihtml_generated)
@@ -43,6 +47,30 @@ apihtml_generated = \
$(addprefix html/libvirt-,$(addsuffix .html,$(modules))) \
$(NULL)
+apiadminhtml = \
+ html/index-admin.html \
+ $(apiadminhtml_generated)
+
+apiadminhtml_generated = \
+ $(addprefix html/libvirt-,$(addsuffix .html,$(modules_admin))) \
+ $(NULL)
+
+apiqemuhtml = \
+ html/index-qemu.html \
+ $(apiqemuhtml_generated)
+
+apiqemuhtml_generated = \
+ $(addprefix html/libvirt-,$(addsuffix .html,$(modules_qemu))) \
+ $(NULL)
+
+apilxchtml = \
+ html/index-lxc.html \
+ $(apilxchtml_generated)
+
+apilxchtml_generated = \
+ $(addprefix html/libvirt-,$(addsuffix .html,$(modules_lxc))) \
+ $(NULL)
+
apipng = \
html/left.png \
html/up.png \
@@ -50,7 +78,7 @@ apipng = \
html/right.png
apirefdir = $(HTML_DIR)/html
-apiref_DATA = $(apihtml) $(apipng)
+apiref_DATA = $(apihtml) $(apiadminhtml) $(apiqemuhtml) $(apilxchtml) $(apipng)
css = \
generic.css \
@@ -188,6 +216,9 @@ aclperms.htmlinc: $(top_srcdir)/src/access/viraccessperm.h \
CLEANFILES = \
$(dot_html) \
$(apihtml) \
+ $(apiadminhtml) \
+ $(apiqemuhtml) \
+ $(apilxchtml) \
$(internals_html) \
$(kbase_html) \
$(api_DATA) \
@@ -252,6 +283,9 @@ EXTRA_DIST += \
|| { rm $@ && exit 1; }
$(apihtml_generated): html/index.html
+$(apiadminhtml_generated): html/index-admin.html
+$(apiqemuhtml_generated): html/index-qemu.html
+$(apilxchtml_generated): html/index-lxc.html
html/index.html: libvirt-api.xml newapi.xsl page.xsl $(APIBUILD_STAMP)
$(AM_V_GEN)$(XSLTPROC) --nonet -o ./ \
@@ -260,6 +294,14 @@ html/index.html: libvirt-api.xml newapi.xsl page.xsl
$(APIBUILD_STAMP)
$(srcdir)/newapi.xsl libvirt-api.xml && \
$(XMLLINT) --nonet --noout html/*.html
+html/index-%.html: libvirt-%-api.xml newapi.xsl page.xsl $(APIBUILD_STAMP)
+ $(AM_V_GEN)$(XSLTPROC) --nonet -o ./ \
+ --stringparam builddir '$(abs_top_builddir)' \
+ --stringparam timestamp $(timestamp) \
+ --stringparam indexfile $(@:html/%=%) \
+ $(srcdir)/newapi.xsl $< && \
+ $(XMLLINT) --nonet --noout html/*.html
+
python_generated_files = \
html/libvirt-libvirt-lxc.html \
html/libvirt-libvirt-qemu.html \
diff --git a/docs/docs.html.in b/docs/docs.html.in
index 6cf09f51bc..268c16f3b3 100644
--- a/docs/docs.html.in
+++ b/docs/docs.html.in
@@ -106,6 +106,10 @@
secret,
storage,
stream
+ and
+ admin,
+ QEMU,
+ LXC libs
Drivers
diff --git a/docs/newapi.xsl b/docs/newapi.xsl
index ca8c703d5c..dd6169397b 100644
--- a/docs/newapi.xsl
+++ b/docs/newapi.xsl
@@ -23,6 +23,7 @@
+
html
@@ -101,10 +102,10 @@
-
- API documentation
-
-The virtualization
API
+
+ API documentation
+
+The virtualization
API
@@ -830,12 +831,12 @@
-
+
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/7] docs: fix ability to view web pages from build tree
Some of the web content is only present in the source tree, thus when viewing pages from the build tree they appear missing. Signed-off-by: Daniel P. Berrangé --- docs/Makefile.am | 24 1 file changed, 24 insertions(+) diff --git a/docs/Makefile.am b/docs/Makefile.am index 50b2fd7066..0c828102e1 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -39,6 +39,30 @@ modules_admin = libvirt-admin modules_qemu = libvirt-qemu modules_lxc = libvirt-lxc +all: vpathhack + +# This hack enables us to view the web pages +# from within the uninstalled build tree +vpathhack: + for dir in fonts js logos; \ + do \ + ln -s $(srcdir)/$$dir $$dir ; \ + done + for file in $(css); \ + do \ + ln -s $(srcdir)/$$file $$file ; \ + done + +clean-local: + for dir in fonts js logos; \ + do \ + rm -f $$dir ; \ + done + for file in $(css); \ + do \ + rm -f $$file ; \ + done + apihtml = \ html/index.html \ $(apihtml_generated) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/7] Drop building of API -refs.xml files
The API cross reference files are not used since
commit d3043afe5ceebd5002bff45db38bb33e090bb4af
Author: Daniel Veillard
Date: Mon Jan 21 08:08:33 2008 +
Remove docs/API*.html
* docs/API* docs/api.xsl docs/site.xsl docs/Makefile.am: remove the
generation of the API*.html files as it's not really useful here
Signed-off-by: Daniel P. Berrangé
---
docs/Makefile.am | 24 +++
docs/apibuild.py | 170 ---
2 files changed, 8 insertions(+), 186 deletions(-)
diff --git a/docs/Makefile.am b/docs/Makefile.am
index 60ca6aba5b..5f59d057f1 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -147,20 +147,16 @@ htmldir = $(HTML_DIR)
html_DATA = $(css) $(png) $(gif) $(dot_html)
xml = \
- libvirt-api.xml \
- libvirt-refs.xml
+ libvirt-api.xml
qemu_xml = \
- libvirt-qemu-api.xml \
- libvirt-qemu-refs.xml
+ libvirt-qemu-api.xml
lxc_xml = \
- libvirt-lxc-api.xml \
- libvirt-lxc-refs.xml
+ libvirt-lxc-api.xml
admin_xml = \
- libvirt-admin-api.xml \
- libvirt-admin-refs.xml
+ libvirt-admin-api.xml
apidir = $(pkgdatadir)/api
api_DATA = \
@@ -220,10 +216,10 @@ timestamp="$(shell if test -n "$$SOURCE_DATE_EPOCH"; \
date -u; \
fi)"
-api: libvirt-api.xml libvirt-refs.xml
-qemu_api: libvirt-qemu-api.xml libvirt-qemu-refs.xml
-lxc_api: libvirt-lxc-api.xml libvirt-lxc-refs.xml
-admin_api: libvirt-admin-api.xml libvirt-admin-refs.xml
+api: libvirt-api.xml
+qemu_api: libvirt-qemu-api.xml
+lxc_api: libvirt-lxc-api.xml
+admin_api: libvirt-admin-api.xml
hvsupport.html: hvsupport.html.in
@@ -285,13 +281,9 @@ python_generated_files = \
html/libvirt-libvirt-admin.html \
html/libvirt-virterror.html \
libvirt-api.xml \
- libvirt-refs.xml \
libvirt-lxc-api.xml \
- libvirt-lxc-refs.xml \
libvirt-qemu-api.xml \
- libvirt-qemu-refs.xml \
libvirt-admin-api.xml \
- libvirt-admin-refs.xml \
$(NULL)
APIBUILD=$(srcdir)/apibuild.py
diff --git a/docs/apibuild.py b/docs/apibuild.py
index dedeb86f3d..5a0224c1c6 100755
--- a/docs/apibuild.py
+++ b/docs/apibuild.py
@@ -2323,166 +2323,6 @@ class docBuilder:
output.write(" \n" %
(id))
output.write("\n")
-def serialize_xrefs_files(self, output):
-headers = sorted(self.headers.keys())
-for file in headers:
-module = self.modulename_file(file)
-output.write("\n" % (module))
-dict = self.headers[file]
-ids = uniq(list(dict.functions.keys()) +
- list(dict.variables.keys()) +
- list(dict.macros.keys()) +
- list(dict.typedefs.keys()) +
- list(dict.structs.keys()) +
- list(dict.enums.keys()))
-for id in ids:
-output.write(" \n" % (id))
-output.write("\n")
-pass
-
-def serialize_xrefs_functions(self, output):
-funcs = {}
-for name in self.idx.functions.keys():
-id = self.idx.functions[name]
-try:
-(ret, params, desc) = id.info
-for param in params:
-if param[0] == 'void':
-continue
-if param[0] in funcs:
-funcs[param[0]].append(name)
-else:
-funcs[param[0]] = [name]
-except Exception:
-pass
-typ = sorted(funcs.keys())
-for type in typ:
-if type in ['', "void", "int", "char *", "const char *"]:
-continue
-output.write("\n" % (type))
-ids = funcs[type]
-ids.sort()
-pid = ''# not sure why we have dups, but get rid of them!
-for id in ids:
-if id != pid:
-output.write(" \n" % (id))
-pid = id
-output.write("\n")
-
-def serialize_xrefs_constructors(self, output):
-funcs = {}
-for name in self.idx.functions.keys():
-id = self.idx.functions[name]
-try:
-(ret, params, desc) = id.info
-if ret[0] == "void":
-continue
-if ret[0] in funcs:
-funcs[ret[0]].append(name)
-else:
-funcs[ret[0]] = [name]
-except Exception:
-pass
-typ = sorted(funcs.keys())
-for type in typ:
-if type in ['', "void", "int", "char *", "const char *"]:
-continue
-output.write("\n" % (type))
-ids = sorted(funcs[type])
-for id in ids:
-output.write(" \n" %
[libvirt] [PATCH 0/7] docs: fix various problems in docs and cleanup rules
There were a few problems in the docs build from the vpath build https://www.redhat.com/archives/libvir-list/2019-November/msg00308.html - We can't browse the website from the build dir - Including of the acl permissions was broken This series fixes those two problems and also fixes the generation of API reference docs for admin, qemu & lxc APIs. Finally there's a bunch of cleanups / simplifications in build rules Daniel P. Berrangé (7): docs: stop using custom rules for building / installing web pages Drop building of API -refs.xml files docs: use variable for referencing API XML filenames docs: generate API reference pages for admin, qemu & lxc libraries docs: remove unused make targets docs: fix ability to view web pages from build tree docs: fix include of ACL permissions files docs/Makefile.am | 180 +++--- docs/apibuild.py | 170 --- docs/docs.html.in | 4 ++ docs/newapi.xsl | 14 ++-- docs/page.xsl | 4 +- 5 files changed, 103 insertions(+), 269 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/7] docs: stop using custom rules for building / installing web pages
Define automake variables for all the data we need built and installed and let automake generate the install rules normally. Signed-off-by: Daniel P. Berrangé --- docs/Makefile.am | 80 +--- 1 file changed, 21 insertions(+), 59 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index 0311bbedd8..60ca6aba5b 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -49,6 +49,9 @@ apipng = \ html/home.png \ html/right.png +apirefdir = $(HTML_DIR)/html +apiref_DATA = $(apihtml) $(apipng) + css = \ generic.css \ libvirt.css \ @@ -59,6 +62,9 @@ javascript = \ js/main.js \ $(NULL) +javascriptdir = $(HTML_DIR)/js +javascript_DATA = $(javascript) + fonts = \ fonts/LICENSE.md \ fonts/stylesheet.css \ @@ -73,6 +79,9 @@ fonts = \ fonts/overpass-mono-semibold.woff \ fonts/overpass-regular.woff +fontsdir = $(HTML_DIR)/fonts +fonts_DATA = $(fonts) + logofiles = \ logos/logo-base.svg \ logos/logo-square.svg \ @@ -92,6 +101,9 @@ logofiles = \ logos/logo-banner-light-256.png \ logos/logo-banner-light-800.png +logofilesdir = $(HTML_DIR)/logos +logofiles_DATA = $(logofiles) + png = \ 32favicon.png \ libvirt-daemon-arch.png \ @@ -107,15 +119,20 @@ gif = \ architecture.gif \ node.gif - internals_html_in = \ $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/internals/*.html.in)) internals_html = $(internals_html_in:%.html.in=%.html) +internalsdir = $(HTML_DIR)/internals +internals_DATA = $(internals_html) + kbase_html_in = \ $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/kbase/*.html.in)) kbase_html = $(kbase_html_in:%.html.in=%.html) +kbasedir = $(HTML_DIR)/kbase +kbase_DATA = $(kbase_html) + # Generate hvsupport.html and news.html first, since they take one extra step. dot_html_generated_in = \ hvsupport.html.in \ @@ -126,6 +143,9 @@ dot_html = \ $(dot_html_generated_in:%.html.in=%.html) \ $(dot_html_in:%.html.in=%.html) +htmldir = $(HTML_DIR) +html_DATA = $(css) $(png) $(gif) $(dot_html) + xml = \ libvirt-api.xml \ libvirt-refs.xml @@ -200,16 +220,11 @@ timestamp="$(shell if test -n "$$SOURCE_DATE_EPOCH"; \ date -u; \ fi)" -all-am: web - api: libvirt-api.xml libvirt-refs.xml qemu_api: libvirt-qemu-api.xml libvirt-qemu-refs.xml lxc_api: libvirt-lxc-api.xml libvirt-lxc-refs.xml admin_api: libvirt-admin-api.xml libvirt-admin-refs.xml -web: $(dot_html) $(internals_html) $(kbase_html) \ - html/index.html - hvsupport.html: hvsupport.html.in hvsupport.html.in: $(srcdir)/hvsupport.pl $(api_DATA) \ @@ -325,56 +340,3 @@ $(APIBUILD_STAMP): $(srcdir)/apibuild.py \ $(AM_V_GEN)srcdir=$(srcdir) builddir=$(builddir) \ $(RUNUTF8) $(PYTHON) $(APIBUILD) touch $@ - - -check-local: all -dist-local: all - -rebuild: api qemu_api lxc_api admin_api all - -install-data-local: - $(mkinstalldirs) $(DESTDIR)$(HTML_DIR) - for f in $(css) $(gif) $(png); do \ - $(INSTALL) -m 0644 $(srcdir)/$$f $(DESTDIR)$(HTML_DIR); done - for f in $(dot_html); do \ - $(INSTALL) -m 0644 $$f $(DESTDIR)$(HTML_DIR); done - $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/js - for f in $(javascript); do \ - $(INSTALL) -m 0644 $(srcdir)/$$f $(DESTDIR)$(HTML_DIR)/js/; done - $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/logos - for f in $(logofiles); do \ - $(INSTALL) -m 0644 $(srcdir)/$$f $(DESTDIR)$(HTML_DIR)/logos; done - $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/html - for h in $(apihtml); do \ - $(INSTALL) -m 0644 $$h $(DESTDIR)$(HTML_DIR)/html; done - for p in $(apipng); do \ - $(INSTALL) -m 0644 $(srcdir)/$$p $(DESTDIR)$(HTML_DIR)/html; done - $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/internals - for f in $(internals_html); do \ - $(INSTALL) -m 0644 $$f $(DESTDIR)$(HTML_DIR)/internals; done - $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/kbase - for f in $(kbase_html); do \ - $(INSTALL) -m 0644 $$f $(DESTDIR)$(HTML_DIR)/kbase; done - $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/fonts - for f in $(fonts); do \ - $(INSTALL) -m 0644 $(srcdir)/$$f $(DESTDIR)$(HTML_DIR)/fonts; \ - done - -uninstall-local: - for f in $(css) $(dot_html) $(gif) $(png) $(fonts); do \ - rm -f $(DESTDIR)$(HTML_DIR)/$$f; \ - done - for f in $(logofiles); do \ - rm -f $(DESTDIR)$(HTML_DIR)/$$f; \ - done - for f in $(javascript); do \ - rm -f $(DESTDIR)$(HTML_DIR)/$$f; \ - done - for h in $(apihtml); do rm -f $(DESTDIR)$(HTML_DIR)/$$h; done - for p in $(apipng); do rm -f $(DESTDIR)$(HTML_DIR)/$$p; done - for f in $(internals_html); do \ - rm -f $(DESTDIR)$(HTML_DIR)/$$f; \ - done - for f in $(kbase_html); do \ - rm -f $(DESTDIR)$(HTML_DIR)/$$f; \ - done -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com
[libvirt] [PATCH 1/4] use g_ascii_strcasecmp instead of c_strcasecmp from gnulib
Signed-off-by: Pavel Hrdina
---
src/internal.h | 4 ++--
tests/virhashtest.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/internal.h b/src/internal.h
index 0ff9f496ac..0780e2a2a3 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -75,9 +75,9 @@
/* String equality tests, suggested by Jim Meyering. */
#define STREQ(a, b) (strcmp(a, b) == 0)
-#define STRCASEEQ(a, b) (c_strcasecmp(a, b) == 0)
+#define STRCASEEQ(a, b) (g_ascii_strcasecmp(a, b) == 0)
#define STRNEQ(a, b) (strcmp(a, b) != 0)
-#define STRCASENEQ(a, b) (c_strcasecmp(a, b) != 0)
+#define STRCASENEQ(a, b) (g_ascii_strcasecmp(a, b) != 0)
#define STREQLEN(a, b, n) (strncmp(a, b, n) == 0)
#define STRCASEEQLEN(a, b, n) (c_strncasecmp(a, b, n) == 0)
#define STRNEQLEN(a, b, n) (strncmp(a, b, n) != 0)
diff --git a/tests/virhashtest.c b/tests/virhashtest.c
index 3132095463..66fa3a428e 100644
--- a/tests/virhashtest.c
+++ b/tests/virhashtest.c
@@ -446,7 +446,7 @@ testHashGetItems(const void *data G_GNUC_UNUSED)
static int
testHashEqualCompValue(const void *value1, const void *value2)
{
-return c_strcasecmp(value1, value2);
+return g_ascii_strcasecmp(value1, value2);
}
static int
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] syntax-check: update strcase check to refer to GLib
Signed-off-by: Pavel Hrdina
---
build-aux/syntax-check.mk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index f1e976ec76..047c48ae83 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -588,7 +588,7 @@ sc_avoid_ctype_macros:
sc_avoid_strcase:
@prohibit='\bstrn?case(cmp|str) *\(' \
- halt='use c-strcase.h instead of raw strcase functions' \
+ halt='use GLib strcase functions instead of raw strcase functions' \
$(_sc_search_regexp)
sc_prohibit_virBufferAdd_with_string_literal:
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] bootstrap.conf: drop usage of c-strcase gnulib module
Signed-off-by: Pavel Hrdina --- bootstrap.conf | 1 - 1 file changed, 1 deletion(-) diff --git a/bootstrap.conf b/bootstrap.conf index abb03bf3a2..d0a39537eb 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -21,7 +21,6 @@ gnulib_modules=' accept bind c-ctype -c-strcase canonicalize-lgpl chown clock-time -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: limit completion of 'domhostname' to active domains
On 11/20/19 10:35 AM, Pino Toscano wrote: Getting the hostname of guest usually requires a in-guest agent, or generally can be determined only on active domains. Signed-off-by: Pino Toscano --- Reviewed-by: Daniel Henrique Barboza -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] use g_ascii_strncasecmp instead of c_strncasecmp from gnulib
Signed-off-by: Pavel Hrdina --- src/internal.h | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/internal.h b/src/internal.h index 0780e2a2a3..3d1e21f0cf 100644 --- a/src/internal.h +++ b/src/internal.h @@ -63,7 +63,6 @@ #include "libvirt/libvirt-admin.h" #include "libvirt/virterror.h" -#include "c-strcase.h" #include "glibcompat.h" /* Merely casting to (void) is not sufficient since the @@ -79,11 +78,11 @@ #define STRNEQ(a, b) (strcmp(a, b) != 0) #define STRCASENEQ(a, b) (g_ascii_strcasecmp(a, b) != 0) #define STREQLEN(a, b, n) (strncmp(a, b, n) == 0) -#define STRCASEEQLEN(a, b, n) (c_strncasecmp(a, b, n) == 0) +#define STRCASEEQLEN(a, b, n) (g_ascii_strncasecmp(a, b, n) == 0) #define STRNEQLEN(a, b, n) (strncmp(a, b, n) != 0) -#define STRCASENEQLEN(a, b, n) (c_strncasecmp(a, b, n) != 0) +#define STRCASENEQLEN(a, b, n) (g_ascii_strncasecmp(a, b, n) != 0) #define STRPREFIX(a, b) (strncmp(a, b, strlen(b)) == 0) -#define STRCASEPREFIX(a, b) (c_strncasecmp(a, b, strlen(b)) == 0) +#define STRCASEPREFIX(a, b) (g_ascii_strncasecmp(a, b, strlen(b)) == 0) #define STRSKIP(a, b) (STRPREFIX(a, b) ? (a) + strlen(b) : NULL) #define STREQ_NULLABLE(a, b) (g_strcmp0(a, b) == 0) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] drop usage of c-strcase gnulib module
Pavel Hrdina (4): use g_ascii_strcasecmp instead of c_strcasecmp from gnulib use g_ascii_strncasecmp instead of c_strncasecmp from gnulib syntax-check: update strcase check to refer to GLib bootstrap.conf: drop usage of c-strcase gnulib module bootstrap.conf| 1 - build-aux/syntax-check.mk | 2 +- src/internal.h| 11 +-- tests/virhashtest.c | 2 +- 4 files changed, 7 insertions(+), 9 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] virt-aa-helper: drop pointer checks in get_files
On Tue, Nov 19, 2019 at 10:31 PM Jamie Strandboge wrote:
>
> On Thu, 14 Nov 2019, Christian Ehrhardt wrote:
>
> > It was mentioned that the pointers in loops like:
> > for (i = 0; i < ctl->def->nserials; i++)
> > if (ctl->def->serials[i] ...
> > will always be !NULL or we would have a much more serious problem.
> >
> > Simplify the if chains in get_files by dropping these checks.
>
> I don't really see why a NULL check is a problem so this feels a bit
> like busy work and it seems short-circuiting and not doing is exactly
> what you would want to do in the event of a 'much more serious problem'.
> Is this really required? +0 to apply on principle (I've not reviewed the
> changes closely).
Well Cole brought it up and I intentionally wanted to have the
discussion separate to the shmem changes.
As I said in the mail on patch 2/2 in v1 of this I consider it
"defensive programming" and not strictly required to be removed.
I'm happy and fine to keep the checks as-is since they are not on a
performance critical path.
I agree as you say "in the event of a 'much more serious problem'"
this would be exactly what we want.
I'll withdraw this patch from the series then unless somebody really
shows a reason to why we should drop the checks.
> > Signed-off-by: Christian Ehrhardt
> > ---
> > src/security/virt-aa-helper.c | 135 --
> > 1 file changed, 63 insertions(+), 72 deletions(-)
> >
> > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> > index c6c4bb9bd0..17f49a6259 100644
> > --- a/src/security/virt-aa-helper.c
> > +++ b/src/security/virt-aa-helper.c
> > @@ -965,8 +965,7 @@ get_files(vahControl * ctl)
> > }
> >
> > for (i = 0; i < ctl->def->nserials; i++)
> > -if (ctl->def->serials[i] &&
> > -(ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
> > ||
> > +if ((ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
> > ||
> > ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV
> > ||
> > ctl->def->serials[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_FILE ||
> > ctl->def->serials[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_UNIX ||
> > @@ -980,8 +979,7 @@ get_files(vahControl * ctl)
> > goto cleanup;
> >
> > for (i = 0; i < ctl->def->nconsoles; i++)
> > -if (ctl->def->consoles[i] &&
> > -(ctl->def->consoles[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_PTY ||
> > +if ((ctl->def->consoles[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_PTY ||
> > ctl->def->consoles[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_DEV ||
> > ctl->def->consoles[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_FILE ||
> > ctl->def->consoles[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_UNIX ||
> > @@ -993,8 +991,7 @@ get_files(vahControl * ctl)
> > goto cleanup;
> >
> > for (i = 0; i < ctl->def->nparallels; i++)
> > -if (ctl->def->parallels[i] &&
> > -(ctl->def->parallels[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_PTY ||
> > +if ((ctl->def->parallels[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_PTY ||
> > ctl->def->parallels[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_DEV ||
> > ctl->def->parallels[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_FILE ||
> > ctl->def->parallels[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_UNIX ||
> > @@ -1008,8 +1005,7 @@ get_files(vahControl * ctl)
> > goto cleanup;
> >
> > for (i = 0; i < ctl->def->nchannels; i++)
> > -if (ctl->def->channels[i] &&
> > -(ctl->def->channels[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_PTY ||
> > +if ((ctl->def->channels[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_PTY ||
> > ctl->def->channels[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_DEV ||
> > ctl->def->channels[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_FILE ||
> > ctl->def->channels[i]->source->type ==
> > VIR_DOMAIN_CHR_TYPE_UNIX ||
> > @@ -1082,76 +1078,74 @@ get_files(vahControl * ctl)
> > "r") != 0)
> > goto cleanup;
> >
> > -for (i = 0; i < ctl->def->nhostdevs; i++)
> > -if (ctl->def->hostdevs[i]) {
> > -virDomainHostdevDefPtr dev = ctl->def->hostdevs[i];
> > -virDomainHostdevSubsysUSBPtr usbsrc =
> > >source.subsys.u.usb;
> > -switch (dev->source.subsys.type) {
> > -case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
> > -virUSBDevicePtr usb =
> > -virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL);
> > +for (i = 0; i < ctl->def->nhostdevs; i++) {
> > +virDomainHostdevDefPtr dev = ctl->def->hostdevs[i];
> > +virDomainHostdevSubsysUSBPtr usbsrc = >source.subsys.u.usb;
> > +switch (dev->source.subsys.type) {
> > +
[libvirt] [PATCH v2 10/14] use g_ascii_isxdigit instead of c_isxdigit from gnulib
Signed-off-by: Pavel Hrdina
---
src/util/virmacaddr.c | 6 +++---
src/util/virutil.c| 2 +-
src/util/viruuid.c| 5 ++---
src/vmx/vmx.c | 2 +-
4 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c
index 3eb6407c45..a6893faf9a 100644
--- a/src/util/virmacaddr.c
+++ b/src/util/virmacaddr.c
@@ -38,9 +38,9 @@ virMacAddrCompare(const char *p, const char *q)
{
unsigned char c, d;
do {
-while (*p == '0' && c_isxdigit(p[1]))
+while (*p == '0' && g_ascii_isxdigit(p[1]))
++p;
-while (*q == '0' && c_isxdigit(q[1]))
+while (*q == '0' && g_ascii_isxdigit(q[1]))
++q;
c = c_tolower(*p);
d = c_tolower(*q);
@@ -153,7 +153,7 @@ virMacAddrParse(const char* str, virMacAddrPtr addr)
/* This is solely to avoid accepting the leading
* space or "+" that strtoul would otherwise accept.
*/
-if (!c_isxdigit(*str))
+if (!g_ascii_isxdigit(*str))
break;
result = strtoul(str, _ptr, 16); /* exempt from syntax-check */
diff --git a/src/util/virutil.c b/src/util/virutil.c
index f3cf494353..12f44796c8 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1525,7 +1525,7 @@ virValidateWWN(const char *wwn)
p += 2;
for (i = 0; p[i]; i++) {
-if (!c_isxdigit(p[i]))
+if (!g_ascii_isxdigit(p[i]))
break;
}
diff --git a/src/util/viruuid.c b/src/util/viruuid.c
index 835c873300..0745189aa9 100644
--- a/src/util/viruuid.c
+++ b/src/util/viruuid.c
@@ -28,7 +28,6 @@
#include
#include
-#include "c-ctype.h"
#include "internal.h"
#include "virutil.h"
#include "virerror.h"
@@ -113,14 +112,14 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid)
cur++;
continue;
}
-if (!c_isxdigit(*cur))
+if (!g_ascii_isxdigit(*cur))
goto error;
uuid[i] = virHexToBin(*cur);
uuid[i] *= 16;
cur++;
if (*cur == 0)
goto error;
-if (!c_isxdigit(*cur))
+if (!g_ascii_isxdigit(*cur))
goto error;
uuid[i] += virHexToBin(*cur);
i++;
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 0ccc4eefe6..20dc974928 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -666,7 +666,7 @@ virVMXUnescapeHex(char *string, char escape)
/* Unescape from 'cXX' where c is the escape char and X is a hex digit */
while (*tmp1 != '\0') {
if (*tmp1 == escape) {
-if (!c_isxdigit(tmp1[1]) || !c_isxdigit(tmp1[2]))
+if (!g_ascii_isxdigit(tmp1[1]) || !g_ascii_isxdigit(tmp1[2]))
return -1;
*tmp2++ = virHexToBin(tmp1[1]) * 16 + virHexToBin(tmp1[2]);
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 09/14] use g_ascii_isspace instead of c_isspace from gnulib
Signed-off-by: Pavel Hrdina
---
src/bhyve/bhyve_parse_command.c| 3 +--
src/conf/nwfilter_conf.c | 3 +--
src/interface/interface_backend_udev.c | 3 +--
src/rpc/virnetsocket.c | 3 +--
src/util/virhostcpu.c | 7 +++
src/util/virnetdevvportprofile.c | 3 +--
src/util/virpidfile.c | 3 +--
src/util/virstoragefile.c | 3 +--
src/util/virstring.c | 6 +++---
src/util/viruuid.c | 4 ++--
10 files changed, 15 insertions(+), 23 deletions(-)
diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
index 050a558cee..5eec05c7d4 100644
--- a/src/bhyve/bhyve_parse_command.c
+++ b/src/bhyve/bhyve_parse_command.c
@@ -31,7 +31,6 @@
#include "virlog.h"
#include "virstring.h"
#include "virutil.h"
-#include "c-ctype.h"
#define VIR_FROM_THIS VIR_FROM_BHYVE
@@ -212,7 +211,7 @@ bhyveCommandLineToArgv(const char *nativeConfig,
arglist[args_count++] = arg;
arglist[args_count] = NULL;
-while (next && c_isspace(*next))
+while (next && g_ascii_isspace(*next))
next++;
curr = next;
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 6c7b85d4c8..5bcf52a84c 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -42,7 +42,6 @@
#include "nwfilter_params.h"
#include "nwfilter_conf.h"
#include "domain_conf.h"
-#include "c-ctype.h"
#include "virfile.h"
#include "virstring.h"
@@ -775,7 +774,7 @@ parseStringItems(const struct int_map *int_map,
i = 0;
while (input[i]) {
found = false;
-while (c_isspace(input[i]) || input[i] == sep)
+while (g_ascii_isspace(input[i]) || input[i] == sep)
i++;
if (!input[i])
break;
diff --git a/src/interface/interface_backend_udev.c
b/src/interface/interface_backend_udev.c
index 29d91d3539..7df5cf2cc3 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -25,7 +25,6 @@
#include "virerror.h"
#include "virfile.h"
-#include "c-ctype.h"
#include "datatypes.h"
#include "domain_conf.h"
#include "interface_driver.h"
@@ -932,7 +931,7 @@ udevGetIfaceDefVlan(struct udev *udev G_GNUC_UNUSED,
vid_pos += strlen(vid_prefix);
if ((vid_len = strspn(vid_pos, "0123456789")) == 0 ||
-!c_isspace(vid_pos[vid_len])) {
+!g_ascii_isspace(vid_pos[vid_len])) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("failed to find the VID for the VLAN device '%s'"),
name);
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 21ef7a8034..b98287e6d7 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -40,7 +40,6 @@
# include
#endif
-#include "c-ctype.h"
#ifdef WITH_SELINUX
# include
#endif
@@ -1813,7 +1812,7 @@ static ssize_t virNetSocketReadWire(virNetSocketPtr sock,
char *buf, size_t len)
errout != NULL) {
size_t elen = strlen(errout);
/* remove trailing whitespace */
-while (elen && c_isspace(errout[elen - 1]))
+while (elen && g_ascii_isspace(errout[elen - 1]))
errout[--elen] = '\0';
}
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index a3ef067f41..22102f2c75 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -38,7 +38,6 @@
# include
#endif
-#include "c-ctype.h"
#include "viralloc.h"
#define LIBVIRT_VIRHOSTCPUPRIV_H_ALLOW
#include "virhostcpupriv.h"
@@ -512,7 +511,7 @@ virHostCPUParseFrequencyString(const char *str,
str += strlen(prefix);
/* Skip all whitespace */
-while (c_isspace(*str))
+while (g_ascii_isspace(*str))
str++;
if (*str == '\0')
goto error;
@@ -524,7 +523,7 @@ virHostCPUParseFrequencyString(const char *str,
str++;
/* Skip all whitespace */
-while (c_isspace(*str))
+while (g_ascii_isspace(*str))
str++;
if (*str == '\0')
goto error;
@@ -533,7 +532,7 @@ virHostCPUParseFrequencyString(const char *str,
* followed by a fractional part (which gets discarded) or some
* leading whitespace */
if (virStrToLong_ui(str, , 10, ) < 0 ||
-(*p != '.' && *p != '\0' && !c_isspace(*p))) {
+(*p != '.' && *p != '\0' && !g_ascii_isspace(*p))) {
goto error;
}
diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
index 6442c2a2cf..a7e7361a4c 100644
--- a/src/util/virnetdevvportprofile.c
+++ b/src/util/virnetdevvportprofile.c
@@ -49,7 +49,6 @@ VIR_ENUM_IMPL(virNetDevVPortProfileOp,
#if WITH_VIRTUALPORT
# include
-# include
# include
# include
@@ -482,7 +481,7 @@ virNetDevVPortProfileGetLldpadPid(void)
char *endptr;
if (virStrToLong_ui(buffer, , 10, ) == 0
-&& (*endptr == '\0' || c_isspace(*endptr))
+&&
[libvirt] [PATCH v2 13/14] syntax-check: update c-type checks to refer to Glib
Signed-off-by: Pavel Hrdina
---
build-aux/syntax-check.mk | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index f1e976ec76..fd8c2c6be5 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -532,7 +532,7 @@ sc_prohibit_select:
# Prohibit the inclusion of .
sc_prohibit_ctype_h:
@prohibit='^# *include *' \
- halt='use c-ctype.h instead of ctype.h' \
+ halt='use Glib g_ascii_* function instead of ctype.h' \
$(_sc_search_regexp)
# We have our own wrapper for mocking purposes
@@ -583,7 +583,7 @@ ctype_re =
isalnum|isalpha|isascii|isblank|iscntrl|isdigit|isgraph|islower\
sc_avoid_ctype_macros:
@prohibit='\b($(ctype_re)) *\(' \
in_vc_files='\.[ch]$$' \
- halt='use c-ctype.h instead of ctype macros' \
+ halt='use Glib g_ascii_ macros instead of ctype macros' \
$(_sc_search_regexp)
sc_avoid_strcase:
@@ -1552,13 +1552,6 @@ sc_prohibit_openat_without_use:
re='\<(openat_(permissive|needs_fchdir|(save|restore)_fail)|l?(stat|ch(own|mod))at|(euid)?accessat|(FCHMOD|FCHOWN|STAT)AT_INLINE)\>'
\
$(_sc_header_without_use)
-# Prohibit the inclusion of c-ctype.h without an actual use.
-ctype_re = isalnum|isalpha|isascii|isblank|iscntrl|isdigit|isgraph|islower\
-|isprint|ispunct|isspace|isupper|isxdigit|tolower|toupper
-sc_prohibit_c_ctype_without_use:
- @h='c-ctype.h' re='\https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 04/14] use g_ascii_isalpha instead of c_isalpha from gnulib
Signed-off-by: Pavel Hrdina
---
src/conf/network_conf.c | 3 +--
src/qemu/qemu_domain.c | 3 +--
src/qemu/qemu_qapi.c| 4 +---
src/util/virconf.c | 2 +-
src/util/virfile.c | 4 ++--
5 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 6970831593..5551b5b4d5 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -37,7 +37,6 @@
#include "virxml.h"
#include "viruuid.h"
#include "virbuffer.h"
-#include "c-ctype.h"
#include "virfile.h"
#include "virstring.h"
@@ -482,7 +481,7 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
}
name = virXMLPropString(node, "name");
-if (name && (!c_isalpha(name[0]))) {
+if (name && (!g_ascii_isalpha(name[0]))) {
virReportError(VIR_ERR_XML_ERROR,
_("Cannot use host name '%s' in network '%s'"),
name, networkName);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f54b9b21ff..5eea0ebcd7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -40,7 +40,6 @@
#include "viralloc.h"
#include "virlog.h"
#include "virerror.h"
-#include "c-ctype.h"
#include "cpu/cpu.h"
#include "viruuid.h"
#include "virfile.h"
@@ -3768,7 +3767,7 @@
qemuDomainDefNamespaceParseCommandlineArgs(qemuDomainXmlNsDefPtr nsdef,
static int
qemuDomainDefNamespaceParseCommandlineEnvNameValidate(const char *envname)
{
-if (!c_isalpha(envname[0]) && envname[0] != '_') {
+if (!g_ascii_isalpha(envname[0]) && envname[0] != '_') {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Invalid environment name, it must begin with a
letter or underscore"));
return -1;
diff --git a/src/qemu/qemu_qapi.c b/src/qemu/qemu_qapi.c
index 484f274c63..15a85fd1ad 100644
--- a/src/qemu/qemu_qapi.c
+++ b/src/qemu/qemu_qapi.c
@@ -25,8 +25,6 @@
#include "virerror.h"
#include "virlog.h"
-#include "c-ctype.h"
-
#define VIR_FROM_THIS VIR_FROM_QEMU
VIR_LOG_INIT("qemu.qemu_qapi");
@@ -165,7 +163,7 @@ virQEMUQAPISchemaTraverseObject(virJSONValuePtr cur,
const char *query = virQEMUQAPISchemaTraverseContextNextQuery(ctxt);
char modifier = *query;
-if (!c_isalpha(modifier))
+if (!g_ascii_isalpha(modifier))
query++;
/* exit on modifers for other types */
diff --git a/src/util/virconf.c b/src/util/virconf.c
index b67716b9ce..a9a7148701 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -556,7 +556,7 @@ virConfParseName(virConfParserCtxtPtr ctxt)
SKIP_BLANKS;
base = ctxt->cur;
/* TODO: probably need encoding support and UTF-8 parsing ! */
-if (!c_isalpha(CUR) &&
+if (!g_ascii_isalpha(CUR) &&
!((ctxt->conf->flags & VIR_CONF_FLAG_VMX_FORMAT) && (CUR == '.'))) {
virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("expecting a name"));
return NULL;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index fca7ff9d35..a329f2e83f 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3190,7 +3190,7 @@ virFileIsAbsPath(const char *path)
return true;
#ifdef WIN32
-if (c_isalpha(path[0]) &&
+if (g_ascii_isalpha(path[0]) &&
path[1] == ':' &&
VIR_FILE_IS_DIR_SEPARATOR(path[2]))
return true;
@@ -3242,7 +3242,7 @@ virFileSkipRoot(const char *path)
#ifdef WIN32
/* Skip X:\ */
-if (c_isalpha(path[0]) &&
+if (g_ascii_isalpha(path[0]) &&
path[1] == ':' &&
VIR_FILE_IS_DIR_SEPARATOR(path[2]))
return path + 3;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 11/14] use g_ascii_tolower instead of c_tolower from gnulib
Signed-off-by: Pavel Hrdina
---
src/remote/remote_driver.c | 3 +--
src/util/virmacaddr.c | 5 ++---
src/util/virutil.c | 5 ++---
src/vmx/vmx.c | 6 ++
tools/virsh-domain.c | 3 +--
tools/vsh.c| 3 +--
6 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index a1384fc655..176400a252 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -49,7 +49,6 @@
#include "virauth.h"
#include "virauthconfig.h"
#include "virstring.h"
-#include "c-ctype.h"
#define VIR_FROM_THIS VIR_FROM_REMOTE
@@ -225,7 +224,7 @@ static int remoteSplitURIScheme(virURIPtr uri,
p = *transport;
while (*p) {
-*p = c_tolower(*p);
+*p = g_ascii_tolower(*p);
p++;
}
}
diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c
index a6893faf9a..182bd582fb 100644
--- a/src/util/virmacaddr.c
+++ b/src/util/virmacaddr.c
@@ -21,7 +21,6 @@
#include
-#include "c-ctype.h"
#include "virmacaddr.h"
#include "virrandom.h"
#include "virutil.h"
@@ -42,8 +41,8 @@ virMacAddrCompare(const char *p, const char *q)
++p;
while (*q == '0' && g_ascii_isxdigit(q[1]))
++q;
-c = c_tolower(*p);
-d = c_tolower(*q);
+c = g_ascii_tolower(*p);
+d = g_ascii_tolower(*q);
if (c == 0 || d == 0)
break;
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 12f44796c8..ed1f696e37 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -63,7 +63,6 @@
# include
#endif
-#include "c-ctype.h"
#include "mgetgroups.h"
#include "virerror.h"
#include "virlog.h"
@@ -201,7 +200,7 @@ virScaleInteger(unsigned long long *value, const char
*suffix,
if (!suffix[1] || STRCASEEQ(suffix + 1, "iB")) {
base = 1024;
-} else if (c_tolower(suffix[1]) == 'b' && !suffix[2]) {
+} else if (g_ascii_tolower(suffix[1]) == 'b' && !suffix[2]) {
base = 1000;
} else {
virReportError(VIR_ERR_INVALID_ARG,
@@ -209,7 +208,7 @@ virScaleInteger(unsigned long long *value, const char
*suffix,
return -1;
}
scale = 1;
-switch (c_tolower(*suffix)) {
+switch (g_ascii_tolower(*suffix)) {
case 'e':
scale *= base;
G_GNUC_FALLTHROUGH;
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 20dc974928..fbf0dd7581 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -22,8 +22,6 @@
#include
-#include
-
#include "internal.h"
#include "virerror.h"
#include "virfile.h"
@@ -1089,7 +1087,7 @@ virVMXHandleLegacySCSIDiskDriverName(virDomainDefPtr def,
tmp = copy;
for (; *tmp != '\0'; ++tmp)
-*tmp = c_tolower(*tmp);
+*tmp = g_ascii_tolower(*tmp);
model = virDomainControllerModelSCSITypeFromString(copy);
VIR_FREE(copy);
@@ -1972,7 +1970,7 @@ virVMXParseSCSIController(virConfPtr conf, int
controller, bool *present,
tmp = virtualDev_string;
for (; *tmp != '\0'; ++tmp)
-*tmp = c_tolower(*tmp);
+*tmp = g_ascii_tolower(*tmp);
*virtualDev =
virVMXControllerModelSCSITypeFromString(virtualDev_string);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b248a15c16..fb7c479f4d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -35,7 +35,6 @@
#include "internal.h"
#include "virbitmap.h"
#include "virbuffer.h"
-#include "c-ctype.h"
#include "conf/domain_conf.h"
#include "viralloc.h"
#include "vircommand.h"
@@ -8780,7 +8779,7 @@ static int getSignalNumber(const char *signame)
char *p = str;
for (i = 0; signame[i]; i++)
-p[i] = c_tolower(signame[i]);
+p[i] = g_ascii_tolower(signame[i]);
if (virStrToLong_i(p, NULL, 10, ) >= 0)
return signum;
diff --git a/tools/vsh.c b/tools/vsh.c
index beee1c2986..6c78a7a373 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -25,7 +25,6 @@
#include
#include
#include
-#include "c-ctype.h"
#include
#include
#include
@@ -2343,7 +2342,7 @@ vshAskReedit(vshControl *ctl, const char *msg, bool
relax_avail)
while (true) {
vshPrint(ctl, "\r%s %s %s: ", msg, _("Try again?"),
relax_avail ? "[y,n,i,f,?]" : "[y,n,f,?]");
-c = c_tolower(getchar());
+c = g_ascii_tolower(getchar());
if (c == '?') {
vshPrintRaw(ctl,
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 12/14] use g_ascii_toupper instead of c_toupper from gnulib
Signed-off-by: Pavel Hrdina
---
src/util/virstring.c | 3 +--
tools/virsh-console.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 311c9058db..bd7c640042 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -21,7 +21,6 @@
#include
#include
-#include "c-ctype.h"
#include "virstring.h"
#include "virthread.h"
#include "viralloc.h"
@@ -1329,7 +1328,7 @@ virStringToUpper(char **dst, const char *src)
return -1;
for (i = 0; src[i]; i++) {
-cap[i] = c_toupper(src[i]);
+cap[i] = g_ascii_toupper(src[i]);
if (cap[i] == '-')
cap[i] = '_';
}
diff --git a/tools/virsh-console.c b/tools/virsh-console.c
index a087d82776..2e498a6903 100644
--- a/tools/virsh-console.c
+++ b/tools/virsh-console.c
@@ -29,7 +29,6 @@
# include
# include
# include
-# include
# include "internal.h"
# include "virsh.h"
@@ -399,7 +398,7 @@ static char
virshGetEscapeChar(const char *s)
{
if (*s == '^')
-return CONTROL(c_toupper(s[1]));
+return CONTROL(g_ascii_toupper(s[1]));
return *s;
}
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 08/14] use g_ascii_isprint instead of c_isprint from gnulib
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_agent.c | 3 +--
src/qemu/qemu_monitor.c | 3 +--
src/util/virstring.c| 4 ++--
tools/vsh-table.c | 3 +--
4 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index e6add56cd6..5fa8d24a91 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -146,7 +146,6 @@ VIR_ONCE_GLOBAL_INIT(qemuAgent);
#if DEBUG_RAW_IO
-# include
static char *
qemuAgentEscapeNonPrintable(const char *text)
{
@@ -155,7 +154,7 @@ qemuAgentEscapeNonPrintable(const char *text)
for (i = 0; text[i] != '\0'; i++) {
if (text[i] == '\\')
virBufferAddLit(, "");
-else if (c_isprint(text[i]) || text[i] == '\n' ||
+else if (g_ascii_isprint(text[i]) || text[i] == '\n' ||
(text[i] == '\r' && text[i+1] == '\n'))
virBufferAddChar(, text[i]);
else
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index cdad5c955d..ae649108d7 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -199,14 +199,13 @@ VIR_ENUM_IMPL(qemuMonitorDumpStatus,
#if DEBUG_RAW_IO
-# include
static char *
qemuMonitorEscapeNonPrintable(const char *text)
{
size_t i;
virBuffer buf = VIR_BUFFER_INITIALIZER;
for (i = 0; text[i] != '\0'; i++) {
-if (c_isprint(text[i]) ||
+if (g_ascii_isprint(text[i]) ||
text[i] == '\n' ||
(text[i] == '\r' && text[i + 1] == '\n'))
virBufferAddChar(, text[i]);
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 40c83841e9..a66b406298 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -1350,7 +1350,7 @@ virStringIsPrintable(const char *str)
size_t i;
for (i = 0; str[i]; i++)
-if (!c_isprint(str[i]))
+if (!g_ascii_isprint(str[i]))
return false;
return true;
@@ -1369,7 +1369,7 @@ virStringBufferIsPrintable(const uint8_t *buf,
size_t i;
for (i = 0; i < buflen; i++)
-if (!c_isprint(buf[i]))
+if (!g_ascii_isprint(buf[i]))
return false;
return true;
diff --git a/tools/vsh-table.c b/tools/vsh-table.c
index 28072c9719..a2365b2c32 100644
--- a/tools/vsh-table.c
+++ b/tools/vsh-table.c
@@ -25,7 +25,6 @@
#include
#include
#include
-#include "c-ctype.h"
#include "viralloc.h"
#include "virbuffer.h"
@@ -244,7 +243,7 @@ vshTableSafeEncode(const char *s, size_t *width)
* Not valid multibyte sequence -- maybe it's
* printable char according to the current locales.
*/
-if (!c_isprint(*p)) {
+if (!g_ascii_isprint(*p)) {
g_snprintf(buf, HEX_ENCODE_LENGTH + 1, "\\x%02x", *p);
buf += HEX_ENCODE_LENGTH;
*width += HEX_ENCODE_LENGTH;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 14/14] bootstrap.conf: drop usage of c-type gnulib module
Signed-off-by: Pavel Hrdina --- bootstrap.conf | 1 - 1 file changed, 1 deletion(-) diff --git a/bootstrap.conf b/bootstrap.conf index abb03bf3a2..e1f715db37 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -20,7 +20,6 @@ gnulib_modules=' accept bind -c-ctype c-strcase canonicalize-lgpl chown -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 07/14] use g_ascii_islower instead of c_islower from gnulib
Signed-off-by: Pavel Hrdina
---
src/util/virutil.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virutil.c b/src/util/virutil.c
index f965bb0e5d..f3cf494353 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -393,11 +393,11 @@ int virDiskNameParse(const char *name, int *disk, int
*partition)
}
}
-if (!ptr || !c_islower(*ptr))
+if (!ptr || !g_ascii_islower(*ptr))
return -1;
for (i = 0; *ptr; i++) {
-if (!c_islower(*ptr))
+if (!g_ascii_islower(*ptr))
break;
idx = (idx + (i < 1 ? 0 : 1)) * 26;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 03/14] use g_ascii_isalnum instead of c_isalnum from gnulib
Signed-off-by: Pavel Hrdina
---
src/node_device/node_device_udev.c | 3 +--
src/util/virconf.c | 2 +-
src/util/virkeyfile.c | 4 ++--
tools/vsh.c| 2 +-
4 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index fabd2ec454..cae00dc9dc 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -22,7 +22,6 @@
#include
#include
#include
-#include
#include "dirname.h"
#include "node_device_conf.h"
@@ -307,7 +306,7 @@ udevGenerateDeviceName(struct udev_device *device,
def->name = virBufferContentAndReset();
for (i = 0; i < strlen(def->name); i++) {
-if (!(c_isalnum(*(def->name + i
+if (!(g_ascii_isalnum(*(def->name + i
*(def->name + i) = '_';
}
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 42c847f999..b67716b9ce 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -562,7 +562,7 @@ virConfParseName(virConfParserCtxtPtr ctxt)
return NULL;
}
while ((ctxt->cur < ctxt->end) &&
- (c_isalnum(CUR) || (CUR == '_') ||
+ (g_ascii_isalnum(CUR) || (CUR == '_') ||
((ctxt->conf->flags & VIR_CONF_FLAG_VMX_FORMAT) &&
((CUR == ':') || (CUR == '.') || (CUR == '-'))) ||
((ctxt->conf->flags & VIR_CONF_FLAG_LXC_FORMAT) &&
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
index 054a187ba2..e50a37d3a7 100644
--- a/src/util/virkeyfile.c
+++ b/src/util/virkeyfile.c
@@ -154,7 +154,7 @@ static int virKeyFileParseValue(virKeyFileParserCtxtPtr
ctxt)
}
keystart = ctxt->cur;
-while (!IS_EOF && c_isalnum(CUR) && CUR != '=')
+while (!IS_EOF && g_ascii_isalnum(CUR) && CUR != '=')
ctxt->cur++;
if (CUR != '=') {
virKeyFileError(ctxt, VIR_ERR_CONF_SYNTAX, "expected end of value
name, expected '='");
@@ -223,7 +223,7 @@ static int virKeyFileParseStatement(virKeyFileParserCtxtPtr
ctxt)
if (CUR == '[') {
ret = virKeyFileParseGroup(ctxt);
-} else if (c_isalnum(CUR)) {
+} else if (g_ascii_isalnum(CUR)) {
ret = virKeyFileParseValue(ctxt);
} else if (CUR == '#' || CUR == ';') {
ret = virKeyFileParseComment(ctxt);
diff --git a/tools/vsh.c b/tools/vsh.c
index 1076c8254b..beee1c2986 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -1444,7 +1444,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser
*parser, vshCmd **partial)
} else if (data_only) {
goto get_data;
} else if (tkdata[0] == '-' && tkdata[1] == '-' &&
- c_isalnum(tkdata[2])) {
+ g_ascii_isalnum(tkdata[2])) {
char *optstr = strchr(tkdata + 2, '=');
size_t opt_index = 0;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 06/14] use g_ascii_isdigit instead of c_isdigit frum gnulib
Signed-off-by: Pavel Hrdina
---
src/interface/interface_backend_udev.c | 2 +-
src/libxl/libxl_conf.c | 3 +--
src/storage/parthelper.c | 5 ++---
src/util/virbitmap.c | 5 ++---
src/util/virconf.c | 7 +++
src/util/virfile.c | 4 +---
6 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/src/interface/interface_backend_udev.c
b/src/interface/interface_backend_udev.c
index b7b06ed67a..29d91d3539 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -567,7 +567,7 @@ udevBridgeScanDirFilter(const struct dirent *entry)
*/
if (strlen(entry->d_name) >= 5) {
if (STRPREFIX(entry->d_name, VIR_NET_GENERATED_TAP_PREFIX) &&
-c_isdigit(entry->d_name[4]))
+g_ascii_isdigit(entry->d_name[4]))
return 0;
}
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 6e4e6f5c90..1097b7a417 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -29,7 +29,6 @@
#include "internal.h"
#include "virlog.h"
#include "virerror.h"
-#include "c-ctype.h"
#include "datatypes.h"
#include "virconf.h"
#include "virfile.h"
@@ -1860,7 +1859,7 @@ libxlDriverGetDom0MaxmemConf(libxlDriverConfigPtr cfg,
char *p = mem_tokens[j] + 4;
unsigned long long multiplier = 1;
-while (c_isdigit(*p))
+while (g_ascii_isdigit(*p))
p++;
if (virStrToLong_ull(mem_tokens[j] + 4, , 10, maxmem) < 0)
break;
diff --git a/src/storage/parthelper.c b/src/storage/parthelper.c
index f1a8cc01ea..761a7f93fc 100644
--- a/src/storage/parthelper.c
+++ b/src/storage/parthelper.c
@@ -38,7 +38,6 @@
#include "virutil.h"
#include "virfile.h"
-#include "c-ctype.h"
#include "virstring.h"
#include "virgettext.h"
@@ -87,7 +86,7 @@ int main(int argc, char **argv)
* path, then append the "p" partition separator. Otherwise, if
* the path ends with a letter already, then no need for a separator.
*/
-if (c_isdigit(path[strlen(path)-1]) || devmap_partsep)
+if (g_ascii_isdigit(path[strlen(path)-1]) || devmap_partsep)
partsep = "p";
else
partsep = "";
@@ -97,7 +96,7 @@ int main(int argc, char **argv)
return 2;
partsep = *canonical_path &&
-c_isdigit(canonical_path[strlen(canonical_path)-1]) ? "p" : "";
+g_ascii_isdigit(canonical_path[strlen(canonical_path)-1]) ? "p" :
"";
}
if ((dev = ped_device_get(path)) == NULL) {
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 9bdb785025..15addee2e9 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -26,7 +26,6 @@
#include "virbitmap.h"
#include "viralloc.h"
#include "virbuffer.h"
-#include "c-ctype.h"
#include "virstring.h"
#include "virutil.h"
#include "virerror.h"
@@ -506,7 +505,7 @@ virBitmapParseSeparator(const char *str,
neg = true;
}
-if (!c_isdigit(*cur))
+if (!g_ascii_isdigit(*cur))
goto error;
if (virStrToLong_i(cur, , 10, ) < 0)
@@ -642,7 +641,7 @@ virBitmapParseUnlimited(const char *str)
neg = true;
}
-if (!c_isdigit(*cur))
+if (!g_ascii_isdigit(*cur))
goto error;
if (virStrToLong_i(cur, , 10, ) < 0)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index a9a7148701..3b45436499 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -29,7 +29,6 @@
#include "virbuffer.h"
#include "virconf.h"
#include "virutil.h"
-#include "c-ctype.h"
#include "virlog.h"
#include "viralloc.h"
#include "virfile.h"
@@ -350,11 +349,11 @@ virConfParseLong(virConfParserCtxtPtr ctxt, long long
*val)
} else if (CUR == '+') {
NEXT;
}
-if ((ctxt->cur >= ctxt->end) || (!c_isdigit(CUR))) {
+if ((ctxt->cur >= ctxt->end) || (!g_ascii_isdigit(CUR))) {
virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("unterminated number"));
return -1;
}
-while ((ctxt->cur < ctxt->end) && (c_isdigit(CUR))) {
+while ((ctxt->cur < ctxt->end) && (g_ascii_isdigit(CUR))) {
l = l * 10 + (CUR - '0');
NEXT;
}
@@ -514,7 +513,7 @@ virConfParseValue(virConfParserCtxtPtr ctxt)
virConfFreeList(lst);
return NULL;
}
-} else if (c_isdigit(CUR) || (CUR == '-') || (CUR == '+')) {
+} else if (g_ascii_isdigit(CUR) || (CUR == '-') || (CUR == '+')) {
if (ctxt->conf->flags & VIR_CONF_FLAG_VMX_FORMAT) {
virConfError(ctxt, VIR_ERR_CONF_SYNTAX,
_("numbers not allowed in VMX format"));
diff --git a/src/util/virfile.c b/src/util/virfile.c
index a329f2e83f..74e1339855 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -80,8 +80,6 @@
#include "virstring.h"
#include
[libvirt] [PATCH v2 02/14] virkeyfile: define IS_ASCII instead c_isascii from gnulib
GLib doesn't provide alternative to c_isascii and this is the only usage
of that macro so define a replacement ourselves.
Signed-off-by: Pavel Hrdina
---
src/util/virkeyfile.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
index 6cd3a5168e..054a187ba2 100644
--- a/src/util/virkeyfile.c
+++ b/src/util/virkeyfile.c
@@ -21,7 +21,6 @@
#include
-#include "c-ctype.h"
#include "virlog.h"
#include "viralloc.h"
#include "virfile.h"
@@ -78,6 +77,7 @@ struct _virKeyFileParserCtxt {
#define IS_EOF (ctxt->cur >= ctxt->end)
#define IS_EOL(c) (((c) == '\n') || ((c) == '\r'))
#define IS_BLANK(c) (((c) == ' ') || ((c) == '\t'))
+#define IS_ASCII(c) ((c) < 128)
#define CUR (*ctxt->cur)
#define NEXT if (!IS_EOF) ctxt->cur++;
@@ -110,7 +110,7 @@ static int virKeyFileParseGroup(virKeyFileParserCtxtPtr
ctxt)
VIR_FREE(ctxt->groupname);
name = ctxt->cur;
-while (!IS_EOF && c_isascii(CUR) && CUR != ']')
+while (!IS_EOF && IS_ASCII(CUR) && CUR != ']')
ctxt->cur++;
if (CUR != ']') {
virKeyFileError(ctxt, VIR_ERR_CONF_SYNTAX, "cannot find end of group
name, expected ']'");
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 05/14] use g_ascii_iscntrl instead of c_iscntrl from gnulib
Signed-off-by: Pavel Hrdina
---
tools/vsh-table.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/vsh-table.c b/tools/vsh-table.c
index 9008b0d431..28072c9719 100644
--- a/tools/vsh-table.c
+++ b/tools/vsh-table.c
@@ -226,7 +226,7 @@ vshTableSafeEncode(const char *s, size_t *width)
while (p && *p) {
if ((*p == '\\' && *(p + 1) == 'x') ||
-c_iscntrl(*p)) {
+g_ascii_iscntrl(*p)) {
g_snprintf(buf, HEX_ENCODE_LENGTH + 1, "\\x%02x", *p);
buf += HEX_ENCODE_LENGTH;
*width += HEX_ENCODE_LENGTH;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 00/14] drop usage of c-type gnulib module
Changes in v2: - fixed missing parentheses in patch 01 - added patch 02 to cover c_isascii Pavel Hrdina (14): util: define IS_BLANK instead of using c_isblank from gnulib virkeyfile: define IS_ASCII instead c_isascii from gnulib use g_ascii_isalnum instead of c_isalnum from gnulib use g_ascii_isalpha instead of c_isalpha from gnulib use g_ascii_iscntrl instead of c_iscntrl from gnulib use g_ascii_isdigit instead of c_isdigit frum gnulib use g_ascii_islower instead of c_islower from gnulib use g_ascii_isprint instead of c_isprint from gnulib use g_ascii_isspace instead of c_isspace from gnulib use g_ascii_isxdigit instead of c_isxdigit from gnulib use g_ascii_tolower instead of c_tolower from gnulib use g_ascii_toupper instead of c_toupper from gnulib syntax-check: update c-type checks to refer to Glib bootstrap.conf: drop usage of c-type gnulib module bootstrap.conf | 1 - build-aux/syntax-check.mk | 11 ++- src/bhyve/bhyve_parse_command.c| 3 +-- src/conf/network_conf.c| 3 +-- src/conf/nwfilter_conf.c | 3 +-- src/interface/interface_backend_udev.c | 5 ++--- src/libxl/libxl_conf.c | 3 +-- src/node_device/node_device_udev.c | 3 +-- src/qemu/qemu_agent.c | 3 +-- src/qemu/qemu_domain.c | 3 +-- src/qemu/qemu_monitor.c| 3 +-- src/qemu/qemu_qapi.c | 4 +--- src/remote/remote_driver.c | 3 +-- src/rpc/virnetsocket.c | 3 +-- src/storage/parthelper.c | 5 ++--- src/util/virbitmap.c | 5 ++--- src/util/virconf.c | 18 +- src/util/virfile.c | 8 +++- src/util/virhostcpu.c | 7 +++ src/util/virkeyfile.c | 13 +++-- src/util/virmacaddr.c | 11 +-- src/util/virnetdevvportprofile.c | 3 +-- src/util/virpidfile.c | 3 +-- src/util/virstoragefile.c | 3 +-- src/util/virstring.c | 13 ++--- src/util/virutil.c | 11 +-- src/util/viruuid.c | 9 - src/vmx/vmx.c | 8 +++- tools/virsh-console.c | 3 +-- tools/virsh-domain.c | 3 +-- tools/vsh-table.c | 5 ++--- tools/vsh.c| 5 ++--- 32 files changed, 73 insertions(+), 111 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 01/14] util: define IS_BLANK instead of using c_isblank from gnulib
The same way how we have IS_EOL in two files where we actually need it
defince IS_BLANK so we can drop usage of c_isblank.
Signed-off-by: Pavel Hrdina
---
src/util/virconf.c| 7 ---
src/util/virkeyfile.c | 5 +++--
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index cc88000387..42c847f999 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -56,13 +56,14 @@ struct _virConfParserCtxt {
#define CUR (*ctxt->cur)
#define NEXT if (ctxt->cur < ctxt->end) ctxt->cur++;
#define IS_EOL(c) (((c) == '\n') || ((c) == '\r'))
+#define IS_BLANK(c) (((c) == ' ') || ((c) == '\t'))
#define SKIP_BLANKS_AND_EOL \
- do { while ((ctxt->cur < ctxt->end) && (c_isblank(CUR) || IS_EOL(CUR))) { \
+ do { while ((ctxt->cur < ctxt->end) && (IS_BLANK(CUR) || IS_EOL(CUR))) { \
if (CUR == '\n') ctxt->line++; \
ctxt->cur++; } } while (0)
#define SKIP_BLANKS \
- do { while ((ctxt->cur < ctxt->end) && (c_isblank(CUR))) \
+ do { while ((ctxt->cur < ctxt->end) && (IS_BLANK(CUR))) \
ctxt->cur++; } while (0)
VIR_ENUM_IMPL(virConf,
@@ -428,7 +429,7 @@ virConfParseString(virConfParserCtxtPtr ctxt)
while ((ctxt->cur < ctxt->end) && (!IS_EOL(CUR)))
NEXT;
/* Reverse to exclude the trailing blanks from the value */
-while ((ctxt->cur > base) && (c_isblank(CUR)))
+while ((ctxt->cur > base) && (IS_BLANK(CUR)))
ctxt->cur--;
if (VIR_STRNDUP(ret, base, ctxt->cur - base) < 0)
return NULL;
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
index c7e756d26a..6cd3a5168e 100644
--- a/src/util/virkeyfile.c
+++ b/src/util/virkeyfile.c
@@ -77,6 +77,7 @@ struct _virKeyFileParserCtxt {
#define IS_EOF (ctxt->cur >= ctxt->end)
#define IS_EOL(c) (((c) == '\n') || ((c) == '\r'))
+#define IS_BLANK(c) (((c) == ' ') || ((c) == '\t'))
#define CUR (*ctxt->cur)
#define NEXT if (!IS_EOF) ctxt->cur++;
@@ -205,7 +206,7 @@ static int virKeyFileParseComment(virKeyFileParserCtxtPtr
ctxt)
static int virKeyFileParseBlank(virKeyFileParserCtxtPtr ctxt)
{
-while ((ctxt->cur < ctxt->end) && c_isblank(CUR))
+while ((ctxt->cur < ctxt->end) && IS_BLANK(CUR))
ctxt->cur++;
if (!((ctxt->cur == ctxt->end) || IS_EOL(CUR))) {
@@ -226,7 +227,7 @@ static int virKeyFileParseStatement(virKeyFileParserCtxtPtr
ctxt)
ret = virKeyFileParseValue(ctxt);
} else if (CUR == '#' || CUR == ';') {
ret = virKeyFileParseComment(ctxt);
-} else if (c_isblank(CUR) || IS_EOL(CUR)) {
+} else if (IS_BLANK(CUR) || IS_EOL(CUR)) {
ret = virKeyFileParseBlank(ctxt);
} else {
virKeyFileError(ctxt, VIR_ERR_CONF_SYNTAX, "unexpected statement");
--
2.23.0
--
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 Tue, Nov 19, 2019 at 10:25 PM Jamie Strandboge wrote:
>
> On Tue, 22 Oct 2019, 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]) {
> > +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;
>
> I'll let others comment on the code changes, but this apparmor rule
> looks ok.
>
> > +} else {
>
> That said, I wonder about the logic here since up above we have:
>
> if (shmem->server.enabled && shmem->server.chr.data.nix.path)
>
> but here we just have 'else'. What happens if server.enabled is false
> and server.chr.data.nix.path is set or vice versa? Does this 'else'
> clause correctly handle those scenarios?
Yes if either of the above isn't fulfilled it will fall back to use
the default paths.
So a single else without other checks should be correct.
The following switch then differs the default paths used base on the model.
>
> > +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;
> > +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;
> > +}
> > +}
> > +}
> > +}
> > +
>
>
> --
> Jamie Strandboge | http://www.canonical.com
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/8] conf: domcaps: Add 'backingStoreInput' domain capability
On Mon, Nov 18, 2019 at 14:54:50 -0500, Cole Robinson wrote: > On 11/18/19 12:02 PM, Peter Krempa wrote: > > Historically we've only supported the as an output-only > > element for domain disks. The documentation states that it may become > > supported on input. To allow management apps detectin once that happens > > add a domain capability which will be asserted if the hypervisor driver > > will be able to obey the as configured on input. > > > > Signed-off-by: Peter Krempa > > --- > > docs/formatdomaincaps.html.in | 7 +++ > > docs/schemas/domaincaps.rng | 9 + > > src/conf/domain_capabilities.c| 1 + > > src/conf/domain_capabilities.h| 1 + > > tests/domaincapsdata/libxl-xenfv.xml | 1 + > > tests/domaincapsdata/libxl-xenpv.xml | 1 + > > Now that I look closer, this patch illustrates one of the issues with > the pattern of virDomainCapsFeaturesInitUnsupported : that function is > used in the bhyve driver, but this patch doesn't update the bhyve test > suite domcaps output. devs can't add a new domcaps feature without > having to regenerate test output for every driver, which is a pain and > easy to overlook. It illustrates my counter-point nicely too. I'd not bother to turn off the capability in bhyve despite it being unsupported. Now at least the tests caught it. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/8] domcaps: Add function for initializing domain caps as unsupported
On Mon, Nov 18, 2019 at 14:43:08 -0500, Cole Robinson wrote:
> On 11/13/19 11:05 AM, Peter Krempa wrote:
> > 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;
> > +}
> > +
> > +
>
> This pattern is suboptimal IMO. Any time a new domcaps capability is
> added, say to serve the qemu/ driver, the developer now needs to
> consider how this shared function impacts libxl domcaps XML.
I see your point, but I think there's merit also towards the other
approach.
Here we explicitly mark everything as unsupported, but that does not
prevent developers from resetting the caps to the missing state in case
when it's ambiguous or they don't wish to deal with other hypervisors.
>
> Say hypothetically tomorrow I want to expose 'acpi' support in domcaps
> . My main goal is to expose this in qemu domcaps. Naturally a
> starting point is to add 'caps->acpi = VIR_TRISTATE_BOOL_NO;' here.
>
> If I don't remember to check libxl code though, I've now potentially
> converted their driver to report blatantly incorrect domcaps output, as
> libxl does support ACPI. If I do remember to check their code, it's my
> responsibility to edit libxl code to ensure it's reporting accurate
> information, which makes my life harder.
In my opinion it's not wrong to ask developers to at least think whether
the change does not influence other hypervisors as well or in case when
it's trivially supported to more than the original intention.
In the end they still can remove it in case it's out of the scope.
Also we should take into account the usability of the caps themselves.
If we by default promote bitrot of other hypervisor drivers we might
miss out on stuff that is actually supported but nobody bothered to wire
up the capability for it.
> With the previous behavior, I could add a new domcap feature to central
> code, fill it in for qemu, and libxl output wouldn't change at all.
> Whether to report supported='no' or supported='yes' is left entirely up
> to libxl driver code. This makes it easier and safer to extend domcaps IMO.
>
> This was the goal of my series to use tristate bool earlier in the year,
> where there's more explanation:
> https://www.redhat.com/archives/libvir-list/2019-March/msg00365.html
I fully agree that we should keep it tristate where possible but also
the most common option will be that the capability is not supported by
other hypervisors when implementing it.
In the end neither approach will get rid of the necessity to do proper
review, but keeping the caps hidden promotes ignorance because the tests
don't catch the possible scope of change.
In the end, we can revert this behaviour (well, it will require some
tweaking now not a straight revert) if you disagree with my reasoning.
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] apparmor: let AppArmorSetSecurityImageLabel append rules
On Tue, Nov 19, 2019 at 10:12 PM Jamie Strandboge wrote: > > On Wed, 16 Oct 2019, Christian Ehrhardt wrote: > > > There are currently broken use cases, e.g. snapshotting more than one disk > > at > > once like: > > $ virsh snapshot-create-as --domain eoan --disk-only --atomic > >--diskspec vda,snapshot=no --diskspec vdb,snapshot=no > >--diskspec vdc,file=/test/disk1.snapshot1.qcow,snapshot=external > >--diskspec vdd,file=/test/disk2.snapshot1.qcow,snapshot=external > > The command above will iterate from qemuDomainSnapshotCreateDiskActive and > > eventually add /test/disk1.snapshot1.qcow first (appears in the rules) > > to then later add /test/disk2.snapshot1.qcow and while doing so throwing > > away the former rule causing it to fail. > > > > All other calls to (re)load_profile already use append=true when adding > > rules append=false is only used when restoring rules [1]. > > > > Fix this by letting AppArmorSetSecurityImageLabel use append=true as well. > > > > Bugs: > > https://bugs.launchpad.net/libvirt/+bug/1845506 > > https://bugzilla.redhat.com/show_bug.cgi?id=1746684 > > > > [1]: https://bugs.launchpad.net/libvirt/+bug/1845506/comments/13 > > > > Signed-off-by: Christian Ehrhardt > > --- > > src/security/security_apparmor.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/security/security_apparmor.c > > b/src/security/security_apparmor.c > > index 320d69e52a..4dd7ba20b4 100644 > > --- a/src/security/security_apparmor.c > > +++ b/src/security/security_apparmor.c > > @@ -812,7 +812,7 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, > > return -1; > > } > > > > -return reload_profile(mgr, def, src->path, false); > > +return reload_profile(mgr, def, src->path, true); > > src/security/security_manager.c shows that > virSecurityManagerSetImageLabel() is called to label 'a storage image > with the configured security label'. Based on your report, it seems that > in addition to the underlying disk (vda in this case), it is also called > for each additional disk (ie, vdc and vdd). Yes in the "transaction" for e.g. a snapshot on multiple disks there will be calls for both disks before any of them becomes part of the VM-definition. That is where the second call "removes" the first rule and then things fail. To be clear, I didn't add any "disk" in the broken use-case it adds further backing image chain elements to existing disks as part of the snapshot action. So if I snapshot in one shot vdb and vdc it is called for each of those adding the new paths for the respective backing chain that grows. If you ever power cycle that it will be part of the definition and virt-aa-helper resolves backing chains as needed. > While your patch to append > makes sense for this scenario, what happens if you update the vm > definition to use 'vde' instead of 'vda', is the vda access removed and > vde added with vdc and vdd remaining? Hmm - not sure what exactly you mean, but lets try to break it ... This splits into three major paths to consider: a) adding/removing disks while the guest is off. This isn't interesting as that way it will be part (or not) of the definition when it starts. The guest will get initial rules based on that definition on start, nothing special. b) adding/removing disks at runtime of a guest b1) you can edit a live definition in XML, but that will only add/modify the disk on next start Even if you now add another disk via proper hot-add later the first will not be added as it isn't really part of the active definition yet. b2) you can hot add/remove a disk, those will be properly labelled and added/removed via their labelling calls on that action c) snapshots at runtime of the guest (that was the broken case) As mentioned above it will need to add new elements to the backing-chain c1) snapshot one disk will work, the call to AppArmorSetSecurityImageLabel will add the new rule needed c2) snapshot multiple disks at once then it will fail without the fix here the second call to AppArmorSetSecurityImageLabel will revoke the former rule Sorry, I don't find the " update the vm definition to use 'vde' instead of 'vda'" that you meant in either of the paths a/b/c above. I'll try to reach you on IRC later on to sort this out ... > What if we remove vda and replace > it with only vde, are vda, vdc and vdd removed? I fear that making this > change will result in scenarios where the rule is (correctly) added, but > previous rules are not removed. > > Can you comment on if this is working correctly? Is it possible to have > tests that demonstrate everything is working as intended? > > -- > Jamie Strandboge | http://www.canonical.com -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/2] qemuProcessStop: Remove image metadata for running mirror jobs
On Wed, Nov 20, 2019 at 15:10:17 +0100, Michal Privoznik wrote:
> On 11/20/19 12:17 PM, Peter Krempa wrote:
> > On Tue, Nov 19, 2019 at 14:15:12 +0100, Michal Privoznik wrote:
> > > If user starts a blockcommit without --pivot or a blockcopy also
> > > without --pivot then we modify access for qemu on both images and
> > > leave it like that until pivot is executed. So far so good.
> > > Problem is, if user instead of issuing pivot calls destroy on the
> > > domain or if qemu dies whilst executing the block job. In this
> > > case we don't ever clear the access we granted at the beginning.
> > > To fix this, we need to mimic what block job code does for
> > > aborting a block job -> remove image metadata from disk->mirror
> > > and disk->src chains.
> > >
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1741456#c19
> > >
> > > Signed-off-by: Michal Privoznik
> > > ---
> > >
> > > Diff to v2:
> > > - updated commit message
> > > - do more remove - for disk->src chain too
> > > - put a comment about the importance of code placement
> > >
> > > src/qemu/qemu_process.c | 12
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > > index 209d07cfe8..b9dd433f54 100644
> > > --- a/src/qemu/qemu_process.c
> > > +++ b/src/qemu/qemu_process.c
> > > @@ -7615,6 +7615,18 @@ void qemuProcessStop(virQEMUDriverPtr driver,
> > > for (i = 0; i < vm->def->niothreadids; i++)
> > > vm->def->iothreadids[i]->thread_id = 0;
> > > +/* Do this explicitly after vm->pid is reset so that security
> > > drivers don't
> > > + * try to enter the domain's namespace which is non-existent by now
> > > as qemu
> > > + * is no longer running. */
> > > +for (i = 0; i < def->ndisks; i++) {
> > > +virDomainDiskDefPtr disk = def->disks[i];
> > > +
> > > +if (disk->mirror)
> > > +qemuBlockRemoveImageMetadata(driver, vm, disk->dst,
> > > disk->mirror);
> > > +
> > > +qemuBlockRemoveImageMetadata(driver, vm, disk->dst, disk->src);
> > > +}
> >
> > So aren't we tracking the image metadata also for the shared subsets of
> > backing chain which might be used by multiple domains?
>
> No. We are not doing that:
>
> static int
> virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr,
> virDomainDefPtr def,
> virStorageSourcePtr src,
> virStorageSourcePtr parent)
> {
> bool remember;
> bool is_toplevel = parent == src || parent->externalDataStore == src;
>
> ...
>
> /* We can't do restore on shared resources safely. Not even
> * with refcounting implemented in XATTRs because if there
> * was a domain running with the feature turned off the
> * refcounter in XATTRs would not reflect the actual number
> * of times the resource is in use and thus the last restore
> * on the resource (which actually restores the original
> * owner) might cut off access to the domain with the feature
> * disabled.
> * For disks, a shared resource is the whole backing chain
> * but the top layer, or read only image, or disk explicitly
> * marked as shared.
> */
> remember = is_toplevel && !src->readonly && !src->shared;
>
> return virSecurityDACSetOwnership(mgr, src, NULL, user, group,
> remember);
> }
>
>
>
> static int
> virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
> virDomainDefPtr def,
> virStorageSourcePtr src,
> virStorageSourcePtr parent)
> {
> bool remember;
> bool is_toplevel = parent == src || parent->externalDataStore == src;
> int ret;
>
> ...
>
> /* We can't do restore on shared resources safely. Not even
> * with refcounting implemented in XATTRs because if there
> * was a domain running with the feature turned off the
> * refcounter in XATTRs would not reflect the actual number
> * of times the resource is in use and thus the last restore
> * on the resource (which actually restores the original
> * owner) might cut off access to the domain with the feature
> * disabled.
> * For disks, a shared resource is the whole backing chain
> * but the top layer, or read only image, or disk explicitly
> * marked as shared.
> */
> remember = is_toplevel && !src->readonly && !src->shared;
>
> ..
>
> ret = virSecuritySELinuxSetFilecon(mgr, src->path, use_label, remember);
>
> ...
> }
>
>
> > Because this
> > undoes everything and we do have some code which does this in the
> > security driver, don't we? So this either is duplicate or it's a
> > superset of what is done in the security driver.
> >
>
> So the problem here is that we call qemuDomainStorageSourceAccessAllow() but
Well,
Re: [libvirt] [PATCH 00/13] drop usage of c-type gnulib module
On Wed, Nov 20, 2019 at 03:07:42PM +0100, Pavel Hrdina wrote: > Pavel Hrdina (13): > util: define IS_BLANK instead of using c_isblank from gnulib > use g_ascii_isalnum instead of c_isalnum from gnulib > use g_ascii_isalpha instead of c_isalpha from gnulib > use g_ascii_iscntrl instead of c_iscntrl from gnulib > use g_ascii_isdigit instead of c_isdigit frum gnulib > use g_ascii_islower instead of c_islower from gnulib > use g_ascii_isprint instead of c_isprint from gnulib > use g_ascii_isspace instead of c_isspace from gnulib > use g_ascii_isxdigit instead of c_isxdigit from gnulib > use g_ascii_tolower instead of c_tolower from gnulib > use g_ascii_toupper instead of c_toupper from gnulib > syntax-check: update c-type checks to refer to Glib > bootstrap.conf: drop usage of c-type gnulib module Ups, I've missed the c_isascii in src/util/virkeyfile.c. I'll post a patch for that shortly. 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 2/4] qemu: block: use the delete flag to delete snapshot images if requested
On Wed, Nov 20, 2019 at 14:56:18 +0100, Pavel Mores wrote:
> On Wed, Nov 20, 2019 at 12:11:32PM +0100, Peter Krempa wrote:
> > On Wed, Nov 20, 2019 at 11:44:53 +0100, Pavel Mores wrote:
> > > When blockcommit finishes successfully, one of the
> > > qemuBlockJobProcessEventCompletedCommit() and
> > > qemuBlockJobProcessEventCompletedActiveCommit() event handlers is called.
> > > This is where the delete flag (stored in qemuBlockJobCommitData since the
> > > previous commit) can actually be used to delete the committed snapshot
> > > images if requested.
> > >
> > > Signed-off-by: Pavel Mores
> > > ---
> > > src/qemu/qemu_blockjob.c | 31 +++
> > > 1 file changed, 31 insertions(+)
> > >
> > > diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> > > index 7d94a6ce38..cf221a2839 100644
> > > --- a/src/qemu/qemu_blockjob.c
> > > +++ b/src/qemu/qemu_blockjob.c
> > > @@ -965,6 +965,31 @@
> > > qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver,
> > > }
> > >
> > >
> > > +/**
> > > + * Helper for qemuBlockJobProcessEventCompletedCommit() and
> > > + * qemuBlockJobProcessEventCompletedActiveCommit(). Relies on
> > > adjustments
> > > + * these functions perform on the 'backingStore' chain to function
> > > correctly.
> > > + *
> > > + * TODO look into removing backing store for non-local snapshots too
> > > + */
> > > +static void
> > > +qemuBlockJobUnlinkCommittedStorage(virStorageSourcePtr top)
> > > +{
> > > +virStorageSourcePtr p = top;
> > > +const size_t errmsg_len = 128;
> > > +char errmsg_buf[errmsg_len];
> > > +char *errmsg;
> > > +
> > > +for (; p != NULL; p = p->backingStore) {
> > > +if (virStorageSourceIsLocalStorage(p))
> >
> > The above condition has a multiline body, so it must be enclosed in a
> > block.
> >
> > > +if (unlink(p->path) < 0) {
> >
> > I was considering whether we should use virFileRemove which will also
> > work properly on root-squashed NFS. I'm not sure though how easy it will
> > be to figure out the correct uid and gid inside this helper.
> >
> > If you are interrested into investigating if it's possible, there should
> > be some prior art in the qemu driver where we try to get the uid/gid frm
> > virStorageSource if it's configured, then fall back to the domain
> > uid/gid of the process.
>
> I can see qemuDomainGetImageIds() in qemu_domain.c which seems somewhat close
yes, this is the one I meant! I couldn't remember :D
> to what you describe. It tries to get uid/gid from the virStorageSourcePtr
> itself, then its parent virStorageSourcePtr, then virDomainObjPtr and
> finally virQEMUDriverConfigPtr.
That is the correct hierarchy. An image file can either have it's own
security label, or the security label of the disk applies to all of the
chain members, if the disk doesn't have a label there is a default label
for the whole VM and in case that isn't available in case of the DAC
driver we apply the configured uid/gid.
> It's static in its file though, and I don't immediately see an easy way to
> pass
> it the virQEMUDriverConfigPtr. However, if it's OK to make it visible from
I think it's okay to export it.
> blockdev code, I could still call it along the lines of
You can get the config via:
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
and driver via:
virQEMUDriverPtr driver = priv->driver;
and finally:
qemuDomainObjPrivatePtr priv = vm->privateData;
Since qemuDomainGetImageIds already takes vm, I'd suggest you refactor
it or add a wrapper which will not require passing of 'cfg', but will
acquire it via the above mentioned method from vm, so that we don't have
to do it in the blockjob code.
> qemuDomainGetImageIds(NULL, vm, p, NULL, , );
>
> as I can apparently get a virDomainObjPtr into the helper quite easily.
>
> > > +errmsg = strerror_r(errno, errmsg_buf, errmsg_len);
> >
> > Please use g_strerror(); It does not require any of the buffers and
> > stuff.
> >
> > > +VIR_WARN("Unable to remove snapshot image file %s (%s)",
> > > + p->path, errmsg);
> > > +}
> > > +}
> >
> > The rest looks good.
> >
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] apparmor: refactor AppArmorSetSecurityImageLabel
On Tue, Nov 19, 2019 at 9:59 PM Jamie Strandboge wrote:
>
> On Wed, 16 Oct 2019, Christian Ehrhardt wrote:
>
> > A lot of the code in AppArmorSetSecurityImageLabel is a duplicate of
> > what is in reload_profile, this refactors AppArmorSetSecurityImageLabel
> > to use reload_profile instead.
> >
> > Signed-off-by: Christian Ehrhardt
> > ---
> > src/security/security_apparmor.c | 38
> > 1 file changed, 9 insertions(+), 29 deletions(-)
> >
> > diff --git a/src/security/security_apparmor.c
> > b/src/security/security_apparmor.c
> > index 691833eb4b..320d69e52a 100644
> > --- a/src/security/security_apparmor.c
> > +++ b/src/security/security_apparmor.c
> > @@ -792,8 +792,6 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> >virStorageSourcePtr src,
> >virSecurityDomainImageLabelFlags flags
> > G_GNUC_UNUSED)
> > {
> > -int rc = -1;
> > -char *profile_name = NULL;
> > virSecurityLabelDefPtr secdef;
> >
> > if (!src->path || !virStorageSourceIsLocalStorage(src))
> > @@ -803,36 +801,18 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr
> > mgr,
> > if (!secdef || !secdef->relabel)
> > return 0;
> >
> > -if (secdef->imagelabel) {
> > -/* if the device doesn't exist, error out */
> > -if (!virFileExists(src->path)) {
> > -virReportError(VIR_ERR_INTERNAL_ERROR,
> > - _("\'%s\' does not exist"),
> > - src->path);
> > -return -1;
> > -}
> > -
> > -if ((profile_name = get_profile_name(def)) == NULL)
> > -return -1;
> > +if (!secdef->imagelabel)
> > +return 0;
> >
> > -/* update the profile only if it is loaded */
> > -if (profile_loaded(secdef->imagelabel) >= 0) {
> > -if (load_profile(mgr, secdef->imagelabel, def,
> > - src->path, false) < 0) {
> > -virReportError(VIR_ERR_INTERNAL_ERROR,
> > - _("cannot update AppArmor profile "
> > - "\'%s\'"),
> > - secdef->imagelabel);
> > -goto cleanup;
> > -}
> > -}
> > +/* if the device doesn't exist, error out */
> > +if (!virFileExists(src->path)) {
> > +virReportError(VIR_ERR_INTERNAL_ERROR,
> > + _("\'%s\' does not exist"),
> > + src->path);
> > +return -1;
> > }
> > -rc = 0;
> >
> > - cleanup:
> > -VIR_FREE(profile_name);
> > -
> > -return rc;
> > +return reload_profile(mgr, def, src->path, false);
>
> The logic of the refactor looks fine, but note by calling
> reload_profile() here, it will call virDomainDefGetSecurityLabelDef()
> which we've already done in this function, so we are adding a needless
> extra call. This doesn't seem to be a particularly expensive call (see
> src/conf/domain_conf.c), so +1 as is
The problem here is that AppArmorSetSecurityImageLabel does an
additional check on "if (!secdef->imagelabel)" which won't be done in
reload_profile.
Therefore without changing behavior (and that was the intention) I
could only remove the first check
Also I looked at the common pattern as an example AppArmorSetFDLabel
also checks !secdef->imagelabel on its own before later calling
reload_profile.
>, though you may want to consider trying to get rid of it.
Yes, might be an opportunity to further streamline later on, but works
fine as-is for now.
Thanks for the review!
>
> --
> Jamie Strandboge | http://www.canonical.com
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/13] use g_ascii_isxdigit instead of c_isxdigit from gnulib
Signed-off-by: Pavel Hrdina
---
src/util/virmacaddr.c | 6 +++---
src/util/virutil.c| 2 +-
src/util/viruuid.c| 5 ++---
src/vmx/vmx.c | 2 +-
4 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c
index 3eb6407c45..a6893faf9a 100644
--- a/src/util/virmacaddr.c
+++ b/src/util/virmacaddr.c
@@ -38,9 +38,9 @@ virMacAddrCompare(const char *p, const char *q)
{
unsigned char c, d;
do {
-while (*p == '0' && c_isxdigit(p[1]))
+while (*p == '0' && g_ascii_isxdigit(p[1]))
++p;
-while (*q == '0' && c_isxdigit(q[1]))
+while (*q == '0' && g_ascii_isxdigit(q[1]))
++q;
c = c_tolower(*p);
d = c_tolower(*q);
@@ -153,7 +153,7 @@ virMacAddrParse(const char* str, virMacAddrPtr addr)
/* This is solely to avoid accepting the leading
* space or "+" that strtoul would otherwise accept.
*/
-if (!c_isxdigit(*str))
+if (!g_ascii_isxdigit(*str))
break;
result = strtoul(str, _ptr, 16); /* exempt from syntax-check */
diff --git a/src/util/virutil.c b/src/util/virutil.c
index f3cf494353..12f44796c8 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1525,7 +1525,7 @@ virValidateWWN(const char *wwn)
p += 2;
for (i = 0; p[i]; i++) {
-if (!c_isxdigit(p[i]))
+if (!g_ascii_isxdigit(p[i]))
break;
}
diff --git a/src/util/viruuid.c b/src/util/viruuid.c
index 835c873300..0745189aa9 100644
--- a/src/util/viruuid.c
+++ b/src/util/viruuid.c
@@ -28,7 +28,6 @@
#include
#include
-#include "c-ctype.h"
#include "internal.h"
#include "virutil.h"
#include "virerror.h"
@@ -113,14 +112,14 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid)
cur++;
continue;
}
-if (!c_isxdigit(*cur))
+if (!g_ascii_isxdigit(*cur))
goto error;
uuid[i] = virHexToBin(*cur);
uuid[i] *= 16;
cur++;
if (*cur == 0)
goto error;
-if (!c_isxdigit(*cur))
+if (!g_ascii_isxdigit(*cur))
goto error;
uuid[i] += virHexToBin(*cur);
i++;
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 0ccc4eefe6..20dc974928 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -666,7 +666,7 @@ virVMXUnescapeHex(char *string, char escape)
/* Unescape from 'cXX' where c is the escape char and X is a hex digit */
while (*tmp1 != '\0') {
if (*tmp1 == escape) {
-if (!c_isxdigit(tmp1[1]) || !c_isxdigit(tmp1[2]))
+if (!g_ascii_isxdigit(tmp1[1]) || !g_ascii_isxdigit(tmp1[2]))
return -1;
*tmp2++ = virHexToBin(tmp1[1]) * 16 + virHexToBin(tmp1[2]);
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/2] qemuProcessStop: Remove image metadata for running mirror jobs
On 11/20/19 12:17 PM, Peter Krempa wrote:
On Tue, Nov 19, 2019 at 14:15:12 +0100, Michal Privoznik wrote:
If user starts a blockcommit without --pivot or a blockcopy also
without --pivot then we modify access for qemu on both images and
leave it like that until pivot is executed. So far so good.
Problem is, if user instead of issuing pivot calls destroy on the
domain or if qemu dies whilst executing the block job. In this
case we don't ever clear the access we granted at the beginning.
To fix this, we need to mimic what block job code does for
aborting a block job -> remove image metadata from disk->mirror
and disk->src chains.
https://bugzilla.redhat.com/show_bug.cgi?id=1741456#c19
Signed-off-by: Michal Privoznik
---
Diff to v2:
- updated commit message
- do more remove - for disk->src chain too
- put a comment about the importance of code placement
src/qemu/qemu_process.c | 12
1 file changed, 12 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 209d07cfe8..b9dd433f54 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7615,6 +7615,18 @@ void qemuProcessStop(virQEMUDriverPtr driver,
for (i = 0; i < vm->def->niothreadids; i++)
vm->def->iothreadids[i]->thread_id = 0;
+/* Do this explicitly after vm->pid is reset so that security drivers don't
+ * try to enter the domain's namespace which is non-existent by now as qemu
+ * is no longer running. */
+for (i = 0; i < def->ndisks; i++) {
+virDomainDiskDefPtr disk = def->disks[i];
+
+if (disk->mirror)
+qemuBlockRemoveImageMetadata(driver, vm, disk->dst, disk->mirror);
+
+qemuBlockRemoveImageMetadata(driver, vm, disk->dst, disk->src);
+}
So aren't we tracking the image metadata also for the shared subsets of
backing chain which might be used by multiple domains?
No. We are not doing that:
static int
virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr,
virDomainDefPtr def,
virStorageSourcePtr src,
virStorageSourcePtr parent)
{
bool remember;
bool is_toplevel = parent == src || parent->externalDataStore == src;
...
/* We can't do restore on shared resources safely. Not even
* with refcounting implemented in XATTRs because if there
* was a domain running with the feature turned off the
* refcounter in XATTRs would not reflect the actual number
* of times the resource is in use and thus the last restore
* on the resource (which actually restores the original
* owner) might cut off access to the domain with the feature
* disabled.
* For disks, a shared resource is the whole backing chain
* but the top layer, or read only image, or disk explicitly
* marked as shared.
*/
remember = is_toplevel && !src->readonly && !src->shared;
return virSecurityDACSetOwnership(mgr, src, NULL, user, group,
remember);
}
static int
virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
virDomainDefPtr def,
virStorageSourcePtr src,
virStorageSourcePtr parent)
{
bool remember;
bool is_toplevel = parent == src || parent->externalDataStore == src;
int ret;
...
/* We can't do restore on shared resources safely. Not even
* with refcounting implemented in XATTRs because if there
* was a domain running with the feature turned off the
* refcounter in XATTRs would not reflect the actual number
* of times the resource is in use and thus the last restore
* on the resource (which actually restores the original
* owner) might cut off access to the domain with the feature
* disabled.
* For disks, a shared resource is the whole backing chain
* but the top layer, or read only image, or disk explicitly
* marked as shared.
*/
remember = is_toplevel && !src->readonly && !src->shared;
..
ret = virSecuritySELinuxSetFilecon(mgr, src->path, use_label,
remember);
...
}
Because this
undoes everything and we do have some code which does this in the
security driver, don't we? So this either is duplicate or it's a
superset of what is done in the security driver.
So the problem here is that we call qemuDomainStorageSourceAccessAllow()
but never call the counter parts. Now, I could call
qemuDomainStorageSourceAccessRevoke() here but:
1) we don't need to bother with CGroups or image locking (as in
virtlockd/sanlock) because qemu process is gone at this point and so is
its CGroup and image locks (both virtlockd and sanlock automatically
release image locks if the process holding locks dies),
2) I'd need to access top_parent and baseSource - are they available
here easily?
3) we are doing this code I'm suggesting already
[libvirt] [PATCH 12/13] syntax-check: update c-type checks to refer to Glib
Signed-off-by: Pavel Hrdina
---
build-aux/syntax-check.mk | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index f1e976ec76..fd8c2c6be5 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -532,7 +532,7 @@ sc_prohibit_select:
# Prohibit the inclusion of .
sc_prohibit_ctype_h:
@prohibit='^# *include *' \
- halt='use c-ctype.h instead of ctype.h' \
+ halt='use Glib g_ascii_* function instead of ctype.h' \
$(_sc_search_regexp)
# We have our own wrapper for mocking purposes
@@ -583,7 +583,7 @@ ctype_re =
isalnum|isalpha|isascii|isblank|iscntrl|isdigit|isgraph|islower\
sc_avoid_ctype_macros:
@prohibit='\b($(ctype_re)) *\(' \
in_vc_files='\.[ch]$$' \
- halt='use c-ctype.h instead of ctype macros' \
+ halt='use Glib g_ascii_ macros instead of ctype macros' \
$(_sc_search_regexp)
sc_avoid_strcase:
@@ -1552,13 +1552,6 @@ sc_prohibit_openat_without_use:
re='\<(openat_(permissive|needs_fchdir|(save|restore)_fail)|l?(stat|ch(own|mod))at|(euid)?accessat|(FCHMOD|FCHOWN|STAT)AT_INLINE)\>'
\
$(_sc_header_without_use)
-# Prohibit the inclusion of c-ctype.h without an actual use.
-ctype_re = isalnum|isalpha|isascii|isblank|iscntrl|isdigit|isgraph|islower\
-|isprint|ispunct|isspace|isupper|isxdigit|tolower|toupper
-sc_prohibit_c_ctype_without_use:
- @h='c-ctype.h' re='\https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/13] bootstrap.conf: drop usage of c-type gnulib module
Signed-off-by: Pavel Hrdina --- bootstrap.conf | 1 - 1 file changed, 1 deletion(-) diff --git a/bootstrap.conf b/bootstrap.conf index abb03bf3a2..e1f715db37 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -20,7 +20,6 @@ gnulib_modules=' accept bind -c-ctype c-strcase canonicalize-lgpl chown -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/13] use g_ascii_isdigit instead of c_isdigit frum gnulib
Signed-off-by: Pavel Hrdina
---
src/interface/interface_backend_udev.c | 2 +-
src/libxl/libxl_conf.c | 3 +--
src/storage/parthelper.c | 5 ++---
src/util/virbitmap.c | 5 ++---
src/util/virconf.c | 7 +++
src/util/virfile.c | 4 +---
6 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/src/interface/interface_backend_udev.c
b/src/interface/interface_backend_udev.c
index b7b06ed67a..29d91d3539 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -567,7 +567,7 @@ udevBridgeScanDirFilter(const struct dirent *entry)
*/
if (strlen(entry->d_name) >= 5) {
if (STRPREFIX(entry->d_name, VIR_NET_GENERATED_TAP_PREFIX) &&
-c_isdigit(entry->d_name[4]))
+g_ascii_isdigit(entry->d_name[4]))
return 0;
}
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 6e4e6f5c90..1097b7a417 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -29,7 +29,6 @@
#include "internal.h"
#include "virlog.h"
#include "virerror.h"
-#include "c-ctype.h"
#include "datatypes.h"
#include "virconf.h"
#include "virfile.h"
@@ -1860,7 +1859,7 @@ libxlDriverGetDom0MaxmemConf(libxlDriverConfigPtr cfg,
char *p = mem_tokens[j] + 4;
unsigned long long multiplier = 1;
-while (c_isdigit(*p))
+while (g_ascii_isdigit(*p))
p++;
if (virStrToLong_ull(mem_tokens[j] + 4, , 10, maxmem) < 0)
break;
diff --git a/src/storage/parthelper.c b/src/storage/parthelper.c
index f1a8cc01ea..761a7f93fc 100644
--- a/src/storage/parthelper.c
+++ b/src/storage/parthelper.c
@@ -38,7 +38,6 @@
#include "virutil.h"
#include "virfile.h"
-#include "c-ctype.h"
#include "virstring.h"
#include "virgettext.h"
@@ -87,7 +86,7 @@ int main(int argc, char **argv)
* path, then append the "p" partition separator. Otherwise, if
* the path ends with a letter already, then no need for a separator.
*/
-if (c_isdigit(path[strlen(path)-1]) || devmap_partsep)
+if (g_ascii_isdigit(path[strlen(path)-1]) || devmap_partsep)
partsep = "p";
else
partsep = "";
@@ -97,7 +96,7 @@ int main(int argc, char **argv)
return 2;
partsep = *canonical_path &&
-c_isdigit(canonical_path[strlen(canonical_path)-1]) ? "p" : "";
+g_ascii_isdigit(canonical_path[strlen(canonical_path)-1]) ? "p" :
"";
}
if ((dev = ped_device_get(path)) == NULL) {
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 9bdb785025..15addee2e9 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -26,7 +26,6 @@
#include "virbitmap.h"
#include "viralloc.h"
#include "virbuffer.h"
-#include "c-ctype.h"
#include "virstring.h"
#include "virutil.h"
#include "virerror.h"
@@ -506,7 +505,7 @@ virBitmapParseSeparator(const char *str,
neg = true;
}
-if (!c_isdigit(*cur))
+if (!g_ascii_isdigit(*cur))
goto error;
if (virStrToLong_i(cur, , 10, ) < 0)
@@ -642,7 +641,7 @@ virBitmapParseUnlimited(const char *str)
neg = true;
}
-if (!c_isdigit(*cur))
+if (!g_ascii_isdigit(*cur))
goto error;
if (virStrToLong_i(cur, , 10, ) < 0)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index cde747158c..dbb3b2c95a 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -29,7 +29,6 @@
#include "virbuffer.h"
#include "virconf.h"
#include "virutil.h"
-#include "c-ctype.h"
#include "virlog.h"
#include "viralloc.h"
#include "virfile.h"
@@ -350,11 +349,11 @@ virConfParseLong(virConfParserCtxtPtr ctxt, long long
*val)
} else if (CUR == '+') {
NEXT;
}
-if ((ctxt->cur >= ctxt->end) || (!c_isdigit(CUR))) {
+if ((ctxt->cur >= ctxt->end) || (!g_ascii_isdigit(CUR))) {
virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("unterminated number"));
return -1;
}
-while ((ctxt->cur < ctxt->end) && (c_isdigit(CUR))) {
+while ((ctxt->cur < ctxt->end) && (g_ascii_isdigit(CUR))) {
l = l * 10 + (CUR - '0');
NEXT;
}
@@ -514,7 +513,7 @@ virConfParseValue(virConfParserCtxtPtr ctxt)
virConfFreeList(lst);
return NULL;
}
-} else if (c_isdigit(CUR) || (CUR == '-') || (CUR == '+')) {
+} else if (g_ascii_isdigit(CUR) || (CUR == '-') || (CUR == '+')) {
if (ctxt->conf->flags & VIR_CONF_FLAG_VMX_FORMAT) {
virConfError(ctxt, VIR_ERR_CONF_SYNTAX,
_("numbers not allowed in VMX format"));
diff --git a/src/util/virfile.c b/src/util/virfile.c
index a329f2e83f..74e1339855 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -80,8 +80,6 @@
#include "virstring.h"
#include
[libvirt] [PATCH 02/13] use g_ascii_isalnum instead of c_isalnum from gnulib
Signed-off-by: Pavel Hrdina
---
src/node_device/node_device_udev.c | 3 +--
src/util/virconf.c | 2 +-
src/util/virkeyfile.c | 4 ++--
tools/vsh.c| 2 +-
4 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index fabd2ec454..cae00dc9dc 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -22,7 +22,6 @@
#include
#include
#include
-#include
#include "dirname.h"
#include "node_device_conf.h"
@@ -307,7 +306,7 @@ udevGenerateDeviceName(struct udev_device *device,
def->name = virBufferContentAndReset();
for (i = 0; i < strlen(def->name); i++) {
-if (!(c_isalnum(*(def->name + i
+if (!(g_ascii_isalnum(*(def->name + i
*(def->name + i) = '_';
}
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 6d03bbd222..305a78b515 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -562,7 +562,7 @@ virConfParseName(virConfParserCtxtPtr ctxt)
return NULL;
}
while ((ctxt->cur < ctxt->end) &&
- (c_isalnum(CUR) || (CUR == '_') ||
+ (g_ascii_isalnum(CUR) || (CUR == '_') ||
((ctxt->conf->flags & VIR_CONF_FLAG_VMX_FORMAT) &&
((CUR == ':') || (CUR == '.') || (CUR == '-'))) ||
((ctxt->conf->flags & VIR_CONF_FLAG_LXC_FORMAT) &&
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
index 6cd3a5168e..1ed5a7ef88 100644
--- a/src/util/virkeyfile.c
+++ b/src/util/virkeyfile.c
@@ -154,7 +154,7 @@ static int virKeyFileParseValue(virKeyFileParserCtxtPtr
ctxt)
}
keystart = ctxt->cur;
-while (!IS_EOF && c_isalnum(CUR) && CUR != '=')
+while (!IS_EOF && g_ascii_isalnum(CUR) && CUR != '=')
ctxt->cur++;
if (CUR != '=') {
virKeyFileError(ctxt, VIR_ERR_CONF_SYNTAX, "expected end of value
name, expected '='");
@@ -223,7 +223,7 @@ static int virKeyFileParseStatement(virKeyFileParserCtxtPtr
ctxt)
if (CUR == '[') {
ret = virKeyFileParseGroup(ctxt);
-} else if (c_isalnum(CUR)) {
+} else if (g_ascii_isalnum(CUR)) {
ret = virKeyFileParseValue(ctxt);
} else if (CUR == '#' || CUR == ';') {
ret = virKeyFileParseComment(ctxt);
diff --git a/tools/vsh.c b/tools/vsh.c
index 1076c8254b..beee1c2986 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -1444,7 +1444,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser
*parser, vshCmd **partial)
} else if (data_only) {
goto get_data;
} else if (tkdata[0] == '-' && tkdata[1] == '-' &&
- c_isalnum(tkdata[2])) {
+ g_ascii_isalnum(tkdata[2])) {
char *optstr = strchr(tkdata + 2, '=');
size_t opt_index = 0;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/13] util: define IS_BLANK instead of using c_isblank from gnulib
The same way how we have IS_EOL in two files where we actually need it
defince IS_BLANK so we can drop usage of c_isblank.
Signed-off-by: Pavel Hrdina
---
src/util/virconf.c| 7 ---
src/util/virkeyfile.c | 5 +++--
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index cc88000387..6d03bbd222 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -56,13 +56,14 @@ struct _virConfParserCtxt {
#define CUR (*ctxt->cur)
#define NEXT if (ctxt->cur < ctxt->end) ctxt->cur++;
#define IS_EOL(c) (((c) == '\n') || ((c) == '\r'))
+#define IS_BLANK(c) (((c) == ' ' || ((c) == '\t'))
#define SKIP_BLANKS_AND_EOL \
- do { while ((ctxt->cur < ctxt->end) && (c_isblank(CUR) || IS_EOL(CUR))) { \
+ do { while ((ctxt->cur < ctxt->end) && (IS_BLANK(CUR) || IS_EOL(CUR))) { \
if (CUR == '\n') ctxt->line++; \
ctxt->cur++; } } while (0)
#define SKIP_BLANKS \
- do { while ((ctxt->cur < ctxt->end) && (c_isblank(CUR))) \
+ do { while ((ctxt->cur < ctxt->end) && (IS_BLANK(CUR))) \
ctxt->cur++; } while (0)
VIR_ENUM_IMPL(virConf,
@@ -428,7 +429,7 @@ virConfParseString(virConfParserCtxtPtr ctxt)
while ((ctxt->cur < ctxt->end) && (!IS_EOL(CUR)))
NEXT;
/* Reverse to exclude the trailing blanks from the value */
-while ((ctxt->cur > base) && (c_isblank(CUR)))
+while ((ctxt->cur > base) && (IS_BLANK(CUR)))
ctxt->cur--;
if (VIR_STRNDUP(ret, base, ctxt->cur - base) < 0)
return NULL;
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
index c7e756d26a..6cd3a5168e 100644
--- a/src/util/virkeyfile.c
+++ b/src/util/virkeyfile.c
@@ -77,6 +77,7 @@ struct _virKeyFileParserCtxt {
#define IS_EOF (ctxt->cur >= ctxt->end)
#define IS_EOL(c) (((c) == '\n') || ((c) == '\r'))
+#define IS_BLANK(c) (((c) == ' ') || ((c) == '\t'))
#define CUR (*ctxt->cur)
#define NEXT if (!IS_EOF) ctxt->cur++;
@@ -205,7 +206,7 @@ static int virKeyFileParseComment(virKeyFileParserCtxtPtr
ctxt)
static int virKeyFileParseBlank(virKeyFileParserCtxtPtr ctxt)
{
-while ((ctxt->cur < ctxt->end) && c_isblank(CUR))
+while ((ctxt->cur < ctxt->end) && IS_BLANK(CUR))
ctxt->cur++;
if (!((ctxt->cur == ctxt->end) || IS_EOL(CUR))) {
@@ -226,7 +227,7 @@ static int virKeyFileParseStatement(virKeyFileParserCtxtPtr
ctxt)
ret = virKeyFileParseValue(ctxt);
} else if (CUR == '#' || CUR == ';') {
ret = virKeyFileParseComment(ctxt);
-} else if (c_isblank(CUR) || IS_EOL(CUR)) {
+} else if (IS_BLANK(CUR) || IS_EOL(CUR)) {
ret = virKeyFileParseBlank(ctxt);
} else {
virKeyFileError(ctxt, VIR_ERR_CONF_SYNTAX, "unexpected statement");
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/13] use g_ascii_isspace instead of c_isspace from gnulib
Signed-off-by: Pavel Hrdina
---
src/bhyve/bhyve_parse_command.c| 3 +--
src/conf/nwfilter_conf.c | 3 +--
src/interface/interface_backend_udev.c | 3 +--
src/rpc/virnetsocket.c | 3 +--
src/util/virhostcpu.c | 7 +++
src/util/virnetdevvportprofile.c | 3 +--
src/util/virpidfile.c | 3 +--
src/util/virstoragefile.c | 3 +--
src/util/virstring.c | 6 +++---
src/util/viruuid.c | 4 ++--
10 files changed, 15 insertions(+), 23 deletions(-)
diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
index 050a558cee..5eec05c7d4 100644
--- a/src/bhyve/bhyve_parse_command.c
+++ b/src/bhyve/bhyve_parse_command.c
@@ -31,7 +31,6 @@
#include "virlog.h"
#include "virstring.h"
#include "virutil.h"
-#include "c-ctype.h"
#define VIR_FROM_THIS VIR_FROM_BHYVE
@@ -212,7 +211,7 @@ bhyveCommandLineToArgv(const char *nativeConfig,
arglist[args_count++] = arg;
arglist[args_count] = NULL;
-while (next && c_isspace(*next))
+while (next && g_ascii_isspace(*next))
next++;
curr = next;
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 6c7b85d4c8..5bcf52a84c 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -42,7 +42,6 @@
#include "nwfilter_params.h"
#include "nwfilter_conf.h"
#include "domain_conf.h"
-#include "c-ctype.h"
#include "virfile.h"
#include "virstring.h"
@@ -775,7 +774,7 @@ parseStringItems(const struct int_map *int_map,
i = 0;
while (input[i]) {
found = false;
-while (c_isspace(input[i]) || input[i] == sep)
+while (g_ascii_isspace(input[i]) || input[i] == sep)
i++;
if (!input[i])
break;
diff --git a/src/interface/interface_backend_udev.c
b/src/interface/interface_backend_udev.c
index 29d91d3539..7df5cf2cc3 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -25,7 +25,6 @@
#include "virerror.h"
#include "virfile.h"
-#include "c-ctype.h"
#include "datatypes.h"
#include "domain_conf.h"
#include "interface_driver.h"
@@ -932,7 +931,7 @@ udevGetIfaceDefVlan(struct udev *udev G_GNUC_UNUSED,
vid_pos += strlen(vid_prefix);
if ((vid_len = strspn(vid_pos, "0123456789")) == 0 ||
-!c_isspace(vid_pos[vid_len])) {
+!g_ascii_isspace(vid_pos[vid_len])) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("failed to find the VID for the VLAN device '%s'"),
name);
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 21ef7a8034..b98287e6d7 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -40,7 +40,6 @@
# include
#endif
-#include "c-ctype.h"
#ifdef WITH_SELINUX
# include
#endif
@@ -1813,7 +1812,7 @@ static ssize_t virNetSocketReadWire(virNetSocketPtr sock,
char *buf, size_t len)
errout != NULL) {
size_t elen = strlen(errout);
/* remove trailing whitespace */
-while (elen && c_isspace(errout[elen - 1]))
+while (elen && g_ascii_isspace(errout[elen - 1]))
errout[--elen] = '\0';
}
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index a3ef067f41..22102f2c75 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -38,7 +38,6 @@
# include
#endif
-#include "c-ctype.h"
#include "viralloc.h"
#define LIBVIRT_VIRHOSTCPUPRIV_H_ALLOW
#include "virhostcpupriv.h"
@@ -512,7 +511,7 @@ virHostCPUParseFrequencyString(const char *str,
str += strlen(prefix);
/* Skip all whitespace */
-while (c_isspace(*str))
+while (g_ascii_isspace(*str))
str++;
if (*str == '\0')
goto error;
@@ -524,7 +523,7 @@ virHostCPUParseFrequencyString(const char *str,
str++;
/* Skip all whitespace */
-while (c_isspace(*str))
+while (g_ascii_isspace(*str))
str++;
if (*str == '\0')
goto error;
@@ -533,7 +532,7 @@ virHostCPUParseFrequencyString(const char *str,
* followed by a fractional part (which gets discarded) or some
* leading whitespace */
if (virStrToLong_ui(str, , 10, ) < 0 ||
-(*p != '.' && *p != '\0' && !c_isspace(*p))) {
+(*p != '.' && *p != '\0' && !g_ascii_isspace(*p))) {
goto error;
}
diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
index 6442c2a2cf..a7e7361a4c 100644
--- a/src/util/virnetdevvportprofile.c
+++ b/src/util/virnetdevvportprofile.c
@@ -49,7 +49,6 @@ VIR_ENUM_IMPL(virNetDevVPortProfileOp,
#if WITH_VIRTUALPORT
# include
-# include
# include
# include
@@ -482,7 +481,7 @@ virNetDevVPortProfileGetLldpadPid(void)
char *endptr;
if (virStrToLong_ui(buffer, , 10, ) == 0
-&& (*endptr == '\0' || c_isspace(*endptr))
+&&
[libvirt] [PATCH 10/13] use g_ascii_tolower instead of c_tolower from gnulib
Signed-off-by: Pavel Hrdina
---
src/remote/remote_driver.c | 3 +--
src/util/virmacaddr.c | 5 ++---
src/util/virutil.c | 5 ++---
src/vmx/vmx.c | 6 ++
tools/virsh-domain.c | 3 +--
tools/vsh.c| 3 +--
6 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index a1384fc655..176400a252 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -49,7 +49,6 @@
#include "virauth.h"
#include "virauthconfig.h"
#include "virstring.h"
-#include "c-ctype.h"
#define VIR_FROM_THIS VIR_FROM_REMOTE
@@ -225,7 +224,7 @@ static int remoteSplitURIScheme(virURIPtr uri,
p = *transport;
while (*p) {
-*p = c_tolower(*p);
+*p = g_ascii_tolower(*p);
p++;
}
}
diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c
index a6893faf9a..182bd582fb 100644
--- a/src/util/virmacaddr.c
+++ b/src/util/virmacaddr.c
@@ -21,7 +21,6 @@
#include
-#include "c-ctype.h"
#include "virmacaddr.h"
#include "virrandom.h"
#include "virutil.h"
@@ -42,8 +41,8 @@ virMacAddrCompare(const char *p, const char *q)
++p;
while (*q == '0' && g_ascii_isxdigit(q[1]))
++q;
-c = c_tolower(*p);
-d = c_tolower(*q);
+c = g_ascii_tolower(*p);
+d = g_ascii_tolower(*q);
if (c == 0 || d == 0)
break;
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 12f44796c8..ed1f696e37 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -63,7 +63,6 @@
# include
#endif
-#include "c-ctype.h"
#include "mgetgroups.h"
#include "virerror.h"
#include "virlog.h"
@@ -201,7 +200,7 @@ virScaleInteger(unsigned long long *value, const char
*suffix,
if (!suffix[1] || STRCASEEQ(suffix + 1, "iB")) {
base = 1024;
-} else if (c_tolower(suffix[1]) == 'b' && !suffix[2]) {
+} else if (g_ascii_tolower(suffix[1]) == 'b' && !suffix[2]) {
base = 1000;
} else {
virReportError(VIR_ERR_INVALID_ARG,
@@ -209,7 +208,7 @@ virScaleInteger(unsigned long long *value, const char
*suffix,
return -1;
}
scale = 1;
-switch (c_tolower(*suffix)) {
+switch (g_ascii_tolower(*suffix)) {
case 'e':
scale *= base;
G_GNUC_FALLTHROUGH;
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 20dc974928..fbf0dd7581 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -22,8 +22,6 @@
#include
-#include
-
#include "internal.h"
#include "virerror.h"
#include "virfile.h"
@@ -1089,7 +1087,7 @@ virVMXHandleLegacySCSIDiskDriverName(virDomainDefPtr def,
tmp = copy;
for (; *tmp != '\0'; ++tmp)
-*tmp = c_tolower(*tmp);
+*tmp = g_ascii_tolower(*tmp);
model = virDomainControllerModelSCSITypeFromString(copy);
VIR_FREE(copy);
@@ -1972,7 +1970,7 @@ virVMXParseSCSIController(virConfPtr conf, int
controller, bool *present,
tmp = virtualDev_string;
for (; *tmp != '\0'; ++tmp)
-*tmp = c_tolower(*tmp);
+*tmp = g_ascii_tolower(*tmp);
*virtualDev =
virVMXControllerModelSCSITypeFromString(virtualDev_string);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b248a15c16..fb7c479f4d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -35,7 +35,6 @@
#include "internal.h"
#include "virbitmap.h"
#include "virbuffer.h"
-#include "c-ctype.h"
#include "conf/domain_conf.h"
#include "viralloc.h"
#include "vircommand.h"
@@ -8780,7 +8779,7 @@ static int getSignalNumber(const char *signame)
char *p = str;
for (i = 0; signame[i]; i++)
-p[i] = c_tolower(signame[i]);
+p[i] = g_ascii_tolower(signame[i]);
if (virStrToLong_i(p, NULL, 10, ) >= 0)
return signum;
diff --git a/tools/vsh.c b/tools/vsh.c
index beee1c2986..6c78a7a373 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -25,7 +25,6 @@
#include
#include
#include
-#include "c-ctype.h"
#include
#include
#include
@@ -2343,7 +2342,7 @@ vshAskReedit(vshControl *ctl, const char *msg, bool
relax_avail)
while (true) {
vshPrint(ctl, "\r%s %s %s: ", msg, _("Try again?"),
relax_avail ? "[y,n,i,f,?]" : "[y,n,f,?]");
-c = c_tolower(getchar());
+c = g_ascii_tolower(getchar());
if (c == '?') {
vshPrintRaw(ctl,
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/13] drop usage of c-type gnulib module
Pavel Hrdina (13): util: define IS_BLANK instead of using c_isblank from gnulib use g_ascii_isalnum instead of c_isalnum from gnulib use g_ascii_isalpha instead of c_isalpha from gnulib use g_ascii_iscntrl instead of c_iscntrl from gnulib use g_ascii_isdigit instead of c_isdigit frum gnulib use g_ascii_islower instead of c_islower from gnulib use g_ascii_isprint instead of c_isprint from gnulib use g_ascii_isspace instead of c_isspace from gnulib use g_ascii_isxdigit instead of c_isxdigit from gnulib use g_ascii_tolower instead of c_tolower from gnulib use g_ascii_toupper instead of c_toupper from gnulib syntax-check: update c-type checks to refer to Glib bootstrap.conf: drop usage of c-type gnulib module bootstrap.conf | 1 - build-aux/syntax-check.mk | 11 ++- src/bhyve/bhyve_parse_command.c| 3 +-- src/conf/network_conf.c| 3 +-- src/conf/nwfilter_conf.c | 3 +-- src/interface/interface_backend_udev.c | 5 ++--- src/libxl/libxl_conf.c | 3 +-- src/node_device/node_device_udev.c | 3 +-- src/qemu/qemu_agent.c | 3 +-- src/qemu/qemu_domain.c | 3 +-- src/qemu/qemu_monitor.c| 3 +-- src/qemu/qemu_qapi.c | 4 +--- src/remote/remote_driver.c | 3 +-- src/rpc/virnetsocket.c | 3 +-- src/storage/parthelper.c | 5 ++--- src/util/virbitmap.c | 5 ++--- src/util/virconf.c | 18 +- src/util/virfile.c | 8 +++- src/util/virhostcpu.c | 7 +++ src/util/virkeyfile.c | 9 + src/util/virmacaddr.c | 11 +-- src/util/virnetdevvportprofile.c | 3 +-- src/util/virpidfile.c | 3 +-- src/util/virstoragefile.c | 3 +-- src/util/virstring.c | 13 ++--- src/util/virutil.c | 11 +-- src/util/viruuid.c | 9 - src/vmx/vmx.c | 8 +++- tools/virsh-console.c | 3 +-- tools/virsh-domain.c | 3 +-- tools/vsh-table.c | 5 ++--- tools/vsh.c| 5 ++--- 32 files changed, 71 insertions(+), 109 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/13] use g_ascii_toupper instead of c_toupper from gnulib
Signed-off-by: Pavel Hrdina
---
src/util/virstring.c | 3 +--
tools/virsh-console.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 311c9058db..bd7c640042 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -21,7 +21,6 @@
#include
#include
-#include "c-ctype.h"
#include "virstring.h"
#include "virthread.h"
#include "viralloc.h"
@@ -1329,7 +1328,7 @@ virStringToUpper(char **dst, const char *src)
return -1;
for (i = 0; src[i]; i++) {
-cap[i] = c_toupper(src[i]);
+cap[i] = g_ascii_toupper(src[i]);
if (cap[i] == '-')
cap[i] = '_';
}
diff --git a/tools/virsh-console.c b/tools/virsh-console.c
index a087d82776..2e498a6903 100644
--- a/tools/virsh-console.c
+++ b/tools/virsh-console.c
@@ -29,7 +29,6 @@
# include
# include
# include
-# include
# include "internal.h"
# include "virsh.h"
@@ -399,7 +398,7 @@ static char
virshGetEscapeChar(const char *s)
{
if (*s == '^')
-return CONTROL(c_toupper(s[1]));
+return CONTROL(g_ascii_toupper(s[1]));
return *s;
}
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/13] use g_ascii_isprint instead of c_isprint from gnulib
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_agent.c | 3 +--
src/qemu/qemu_monitor.c | 3 +--
src/util/virstring.c| 4 ++--
tools/vsh-table.c | 3 +--
4 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index e6add56cd6..5fa8d24a91 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -146,7 +146,6 @@ VIR_ONCE_GLOBAL_INIT(qemuAgent);
#if DEBUG_RAW_IO
-# include
static char *
qemuAgentEscapeNonPrintable(const char *text)
{
@@ -155,7 +154,7 @@ qemuAgentEscapeNonPrintable(const char *text)
for (i = 0; text[i] != '\0'; i++) {
if (text[i] == '\\')
virBufferAddLit(, "");
-else if (c_isprint(text[i]) || text[i] == '\n' ||
+else if (g_ascii_isprint(text[i]) || text[i] == '\n' ||
(text[i] == '\r' && text[i+1] == '\n'))
virBufferAddChar(, text[i]);
else
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index cdad5c955d..ae649108d7 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -199,14 +199,13 @@ VIR_ENUM_IMPL(qemuMonitorDumpStatus,
#if DEBUG_RAW_IO
-# include
static char *
qemuMonitorEscapeNonPrintable(const char *text)
{
size_t i;
virBuffer buf = VIR_BUFFER_INITIALIZER;
for (i = 0; text[i] != '\0'; i++) {
-if (c_isprint(text[i]) ||
+if (g_ascii_isprint(text[i]) ||
text[i] == '\n' ||
(text[i] == '\r' && text[i + 1] == '\n'))
virBufferAddChar(, text[i]);
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 40c83841e9..a66b406298 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -1350,7 +1350,7 @@ virStringIsPrintable(const char *str)
size_t i;
for (i = 0; str[i]; i++)
-if (!c_isprint(str[i]))
+if (!g_ascii_isprint(str[i]))
return false;
return true;
@@ -1369,7 +1369,7 @@ virStringBufferIsPrintable(const uint8_t *buf,
size_t i;
for (i = 0; i < buflen; i++)
-if (!c_isprint(buf[i]))
+if (!g_ascii_isprint(buf[i]))
return false;
return true;
diff --git a/tools/vsh-table.c b/tools/vsh-table.c
index 28072c9719..a2365b2c32 100644
--- a/tools/vsh-table.c
+++ b/tools/vsh-table.c
@@ -25,7 +25,6 @@
#include
#include
#include
-#include "c-ctype.h"
#include "viralloc.h"
#include "virbuffer.h"
@@ -244,7 +243,7 @@ vshTableSafeEncode(const char *s, size_t *width)
* Not valid multibyte sequence -- maybe it's
* printable char according to the current locales.
*/
-if (!c_isprint(*p)) {
+if (!g_ascii_isprint(*p)) {
g_snprintf(buf, HEX_ENCODE_LENGTH + 1, "\\x%02x", *p);
buf += HEX_ENCODE_LENGTH;
*width += HEX_ENCODE_LENGTH;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/13] use g_ascii_islower instead of c_islower from gnulib
Signed-off-by: Pavel Hrdina
---
src/util/virutil.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virutil.c b/src/util/virutil.c
index f965bb0e5d..f3cf494353 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -393,11 +393,11 @@ int virDiskNameParse(const char *name, int *disk, int
*partition)
}
}
-if (!ptr || !c_islower(*ptr))
+if (!ptr || !g_ascii_islower(*ptr))
return -1;
for (i = 0; *ptr; i++) {
-if (!c_islower(*ptr))
+if (!g_ascii_islower(*ptr))
break;
idx = (idx + (i < 1 ? 0 : 1)) * 26;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/13] use g_ascii_isalpha instead of c_isalpha from gnulib
Signed-off-by: Pavel Hrdina
---
src/conf/network_conf.c | 3 +--
src/qemu/qemu_domain.c | 3 +--
src/qemu/qemu_qapi.c| 4 +---
src/util/virconf.c | 2 +-
src/util/virfile.c | 4 ++--
5 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 6970831593..5551b5b4d5 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -37,7 +37,6 @@
#include "virxml.h"
#include "viruuid.h"
#include "virbuffer.h"
-#include "c-ctype.h"
#include "virfile.h"
#include "virstring.h"
@@ -482,7 +481,7 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
}
name = virXMLPropString(node, "name");
-if (name && (!c_isalpha(name[0]))) {
+if (name && (!g_ascii_isalpha(name[0]))) {
virReportError(VIR_ERR_XML_ERROR,
_("Cannot use host name '%s' in network '%s'"),
name, networkName);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f54b9b21ff..5eea0ebcd7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -40,7 +40,6 @@
#include "viralloc.h"
#include "virlog.h"
#include "virerror.h"
-#include "c-ctype.h"
#include "cpu/cpu.h"
#include "viruuid.h"
#include "virfile.h"
@@ -3768,7 +3767,7 @@
qemuDomainDefNamespaceParseCommandlineArgs(qemuDomainXmlNsDefPtr nsdef,
static int
qemuDomainDefNamespaceParseCommandlineEnvNameValidate(const char *envname)
{
-if (!c_isalpha(envname[0]) && envname[0] != '_') {
+if (!g_ascii_isalpha(envname[0]) && envname[0] != '_') {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Invalid environment name, it must begin with a
letter or underscore"));
return -1;
diff --git a/src/qemu/qemu_qapi.c b/src/qemu/qemu_qapi.c
index 484f274c63..15a85fd1ad 100644
--- a/src/qemu/qemu_qapi.c
+++ b/src/qemu/qemu_qapi.c
@@ -25,8 +25,6 @@
#include "virerror.h"
#include "virlog.h"
-#include "c-ctype.h"
-
#define VIR_FROM_THIS VIR_FROM_QEMU
VIR_LOG_INIT("qemu.qemu_qapi");
@@ -165,7 +163,7 @@ virQEMUQAPISchemaTraverseObject(virJSONValuePtr cur,
const char *query = virQEMUQAPISchemaTraverseContextNextQuery(ctxt);
char modifier = *query;
-if (!c_isalpha(modifier))
+if (!g_ascii_isalpha(modifier))
query++;
/* exit on modifers for other types */
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 305a78b515..cde747158c 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -556,7 +556,7 @@ virConfParseName(virConfParserCtxtPtr ctxt)
SKIP_BLANKS;
base = ctxt->cur;
/* TODO: probably need encoding support and UTF-8 parsing ! */
-if (!c_isalpha(CUR) &&
+if (!g_ascii_isalpha(CUR) &&
!((ctxt->conf->flags & VIR_CONF_FLAG_VMX_FORMAT) && (CUR == '.'))) {
virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("expecting a name"));
return NULL;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index fca7ff9d35..a329f2e83f 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3190,7 +3190,7 @@ virFileIsAbsPath(const char *path)
return true;
#ifdef WIN32
-if (c_isalpha(path[0]) &&
+if (g_ascii_isalpha(path[0]) &&
path[1] == ':' &&
VIR_FILE_IS_DIR_SEPARATOR(path[2]))
return true;
@@ -3242,7 +3242,7 @@ virFileSkipRoot(const char *path)
#ifdef WIN32
/* Skip X:\ */
-if (c_isalpha(path[0]) &&
+if (g_ascii_isalpha(path[0]) &&
path[1] == ':' &&
VIR_FILE_IS_DIR_SEPARATOR(path[2]))
return path + 3;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/13] use g_ascii_iscntrl instead of c_iscntrl from gnulib
Signed-off-by: Pavel Hrdina
---
tools/vsh-table.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/vsh-table.c b/tools/vsh-table.c
index 9008b0d431..28072c9719 100644
--- a/tools/vsh-table.c
+++ b/tools/vsh-table.c
@@ -226,7 +226,7 @@ vshTableSafeEncode(const char *s, size_t *width)
while (p && *p) {
if ((*p == '\\' && *(p + 1) == 'x') ||
-c_iscntrl(*p)) {
+g_ascii_iscntrl(*p)) {
g_snprintf(buf, HEX_ENCODE_LENGTH + 1, "\\x%02x", *p);
buf += HEX_ENCODE_LENGTH;
*width += HEX_ENCODE_LENGTH;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 14/23] src: rewrite remote protocol checker in Python
On Wed, Nov 20, 2019 at 02:13:37PM +0100, Ján Tomko wrote:
> On Mon, Nov 11, 2019 at 02:38:17PM +, Daniel P. Berrangé wrote:
> > As part of an goal to eliminate Perl from libvirt build tools,
> > rewrite the pdwtags processing script in Python.
> >
> > The original inline shell and perl code was completely
> > unintelligible. The new python code is a manual conversion
> > that attempts todo basically the same thing.
> >
> > Signed-off-by: Daniel P. Berrangé
> > ---
> > Makefile.am | 1 +
> > build-aux/syntax-check.mk| 3 +-
> > scripts/check-remote-protocol.py | 136 +++
> > src/Makefile.am | 98 --
> > 4 files changed, 155 insertions(+), 83 deletions(-)
> > create mode 100644 scripts/check-remote-protocol.py
> >
> > +if n < 1:
> > +print("WARNING: No structs/enums matched. Your", file=sys.stderr)
> > +print("WARNING: pdwtags program is probably too old", file=sys.stderr)
> > +print("WARNING: skipping the remote protocol test", file=sys.stderr)
> > +print("WARNING: install dwarves-1.3 or newer", file=sys.stderr)
> > +sys.exit(8)
> > +
> > +diff = subprocess.Popen(["diff", "-u", expected, "-"],
> > stdin=subprocess.PIPE)
> > +actualstr = "\n".join(actual) + "\n"
> > +diff.communicate(input=actualstr.encode("utf-8"))
> > +
> > +sys.exit(diff.returncode)
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index bb63e2486c..c40e61e7ee 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -196,83 +196,6 @@ DRIVER_SOURCES += \
> >
> >
> >
> > -# Ensure that we don't change the struct or member names or member ordering
> > -# in remote_protocol.x The embedded perl below needs a few comments, and
> > -# presumes you know what pdwtags output looks like:
> > -# * use -0777 -n to slurp the entire file into $_.
> > -# * the "split" splits on the /* DD */ comments, so that $p iterates
> > -# through the struct definitions.
> > -# * process only "struct remote_..." entries
> > -# * remove comments and preceding TAB throughout
> > -# * remove empty lines throughout
> > -# * remove white space at end of buffer
> > -
> > -# With pdwtags 1.8, --verbose output includes separators like these:
> > -# /* 93 */
> > -# /* <0> (null):0 */
> > -# with the second line omitted for intrinsic types.
> > -# Whereas with pdwtags 1.3, they look like this:
> > -# /* <2d2> /usr/include/libio.h:180 */
> > -# The alternation of the following regexps matches both cases.
> > -r1 = /\* \d+ \*/
> > -r2 = /\* <[[:xdigit:]]+> \S+:\d+ \*/
> > -libs_prefix = remote_|qemu_|lxc_|admin_
> > -other_prefix = keepalive|vir(Net|LockSpace|LXCMonitor)
> > -struct_prefix = ($(libs_prefix)|$(other_prefix))
> > -
> > -# Depending on configure options, libtool creates one or both of
> > -# remote/{,.libs/}libvirt_driver_remote_la-remote_protocol.o. We want
> > -# the newest of the two, in case configure options changed and a stale
> > -# file is left around from an earlier build.
>
> So I assume the configure option that gives you the path without .libs
> is gone?
I honestly don't know what configure option it might have been
referring to in the first place.
My guess was perhaps related to --disable-shared to force a static
library build. Libvirt fails to link when doing this and we never
test it, so I considerd static build out of scope, and in any case
when i try it a .libs dir still appears
In absence of a clear need I figured just ignore this comment and
lets see if anyone reports breakage that our CI systems don't
catch.
> > -# The pdwtags output is completely different when building with clang
> > -# which causes the comparison against expected output to fail, so skip
> > -# if using clang as CC.
> > -PDWTAGS = \
> > - $(AM_V_GEN)if $(CC) -v 2>&1 | grep -q clang; then \
> > - echo 'WARNING: skipping pdwtags test with Clang' >&2; \
> > - exit 0; \
> > - fi; \
> > - if (pdwtags --help) > /dev/null 2>&1; then \
> > - o=`ls -t $(<:.lo=.$(OBJEXT)) \
> > -$(subst /,/.libs/,$(<:.lo=.$(OBJEXT))) \
> > - 2>/dev/null | sed -n 1p`; \
>
> And that OBJEXT is always .o
Same note as above.
>
> > - test -f "$$o" || { echo ".o for $< not found" >&2; exit 1; }; \
> > - pdwtags --verbose $$o > $(@F)-t1 2> $(@F)-t2; \
> > - if test ! -s $(@F)-t1 && test -s $(@F)-t2; then \
> > - rm -rf $(@F)-t?; \
> > - echo 'WARNING: pdwtags appears broken; skipping the $@ test' >&2;\
> > - else \
> > - $(PERL) -0777 -n \
> > - -e 'foreach my $$p (split m!\n*(?:$(r1)|$(r2))\n!) {' \
> > - -e ' if ($$p =~ /^(struct|enum) $(struct_prefix)/) {' \
> > - -e '$$p =~ s!\t*/\*.*?\*/!!sg;' \
> > - -e '$$p =~ s!\s+\n!\n!sg;' \
> > - -e '$$p =~ s!\s+$$!!;' \
> > - -e '$$p =~ s!\t!!g;' \
> > - -e 'print "$$p\n";' \
> > - -e '$$n++;' \
> > - -e ' }' \
> > -
Re: [libvirt] [PATCH v2 2/4] qemu: block: use the delete flag to delete snapshot images if requested
On Wed, Nov 20, 2019 at 12:11:32PM +0100, Peter Krempa wrote:
> On Wed, Nov 20, 2019 at 11:44:53 +0100, Pavel Mores wrote:
> > When blockcommit finishes successfully, one of the
> > qemuBlockJobProcessEventCompletedCommit() and
> > qemuBlockJobProcessEventCompletedActiveCommit() event handlers is called.
> > This is where the delete flag (stored in qemuBlockJobCommitData since the
> > previous commit) can actually be used to delete the committed snapshot
> > images if requested.
> >
> > Signed-off-by: Pavel Mores
> > ---
> > src/qemu/qemu_blockjob.c | 31 +++
> > 1 file changed, 31 insertions(+)
> >
> > diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> > index 7d94a6ce38..cf221a2839 100644
> > --- a/src/qemu/qemu_blockjob.c
> > +++ b/src/qemu/qemu_blockjob.c
> > @@ -965,6 +965,31 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr
> > driver,
> > }
> >
> >
> > +/**
> > + * Helper for qemuBlockJobProcessEventCompletedCommit() and
> > + * qemuBlockJobProcessEventCompletedActiveCommit(). Relies on adjustments
> > + * these functions perform on the 'backingStore' chain to function
> > correctly.
> > + *
> > + * TODO look into removing backing store for non-local snapshots too
> > + */
> > +static void
> > +qemuBlockJobUnlinkCommittedStorage(virStorageSourcePtr top)
> > +{
> > +virStorageSourcePtr p = top;
> > +const size_t errmsg_len = 128;
> > +char errmsg_buf[errmsg_len];
> > +char *errmsg;
> > +
> > +for (; p != NULL; p = p->backingStore) {
> > +if (virStorageSourceIsLocalStorage(p))
>
> The above condition has a multiline body, so it must be enclosed in a
> block.
>
> > +if (unlink(p->path) < 0) {
>
> I was considering whether we should use virFileRemove which will also
> work properly on root-squashed NFS. I'm not sure though how easy it will
> be to figure out the correct uid and gid inside this helper.
>
> If you are interrested into investigating if it's possible, there should
> be some prior art in the qemu driver where we try to get the uid/gid frm
> virStorageSource if it's configured, then fall back to the domain
> uid/gid of the process.
I can see qemuDomainGetImageIds() in qemu_domain.c which seems somewhat close
to what you describe. It tries to get uid/gid from the virStorageSourcePtr
itself, then its parent virStorageSourcePtr, then virDomainObjPtr and
finally virQEMUDriverConfigPtr.
It's static in its file though, and I don't immediately see an easy way to pass
it the virQEMUDriverConfigPtr. However, if it's OK to make it visible from
blockdev code, I could still call it along the lines of
qemuDomainGetImageIds(NULL, vm, p, NULL, , );
as I can apparently get a virDomainObjPtr into the helper quite easily.
> > +errmsg = strerror_r(errno, errmsg_buf, errmsg_len);
>
> Please use g_strerror(); It does not require any of the buffers and
> stuff.
>
> > +VIR_WARN("Unable to remove snapshot image file %s (%s)",
> > + p->path, errmsg);
> > +}
> > +}
>
> The rest looks good.
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: limit completion of 'domhostname' to active domains
Getting the hostname of guest usually requires a in-guest agent, or
generally can be determined only on active domains.
Signed-off-by: Pino Toscano
---
tools/virsh-domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b248a15c16..6be9780836 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11668,7 +11668,7 @@ static const vshCmdInfo info_domhostname[] = {
};
static const vshCmdOptDef opts_domhostname[] = {
-VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = NULL}
};
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 14/23] src: rewrite remote protocol checker in Python
On Mon, Nov 11, 2019 at 02:38:17PM +, Daniel P. Berrangé wrote:
As part of an goal to eliminate Perl from libvirt build tools,
rewrite the pdwtags processing script in Python.
The original inline shell and perl code was completely
unintelligible. The new python code is a manual conversion
that attempts todo basically the same thing.
Signed-off-by: Daniel P. Berrangé
---
Makefile.am | 1 +
build-aux/syntax-check.mk| 3 +-
scripts/check-remote-protocol.py | 136 +++
src/Makefile.am | 98 --
4 files changed, 155 insertions(+), 83 deletions(-)
create mode 100644 scripts/check-remote-protocol.py
+if n < 1:
+print("WARNING: No structs/enums matched. Your", file=sys.stderr)
+print("WARNING: pdwtags program is probably too old", file=sys.stderr)
+print("WARNING: skipping the remote protocol test", file=sys.stderr)
+print("WARNING: install dwarves-1.3 or newer", file=sys.stderr)
+sys.exit(8)
+
+diff = subprocess.Popen(["diff", "-u", expected, "-"], stdin=subprocess.PIPE)
+actualstr = "\n".join(actual) + "\n"
+diff.communicate(input=actualstr.encode("utf-8"))
+
+sys.exit(diff.returncode)
diff --git a/src/Makefile.am b/src/Makefile.am
index bb63e2486c..c40e61e7ee 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -196,83 +196,6 @@ DRIVER_SOURCES += \
-# Ensure that we don't change the struct or member names or member ordering
-# in remote_protocol.x The embedded perl below needs a few comments, and
-# presumes you know what pdwtags output looks like:
-# * use -0777 -n to slurp the entire file into $_.
-# * the "split" splits on the /* DD */ comments, so that $p iterates
-# through the struct definitions.
-# * process only "struct remote_..." entries
-# * remove comments and preceding TAB throughout
-# * remove empty lines throughout
-# * remove white space at end of buffer
-
-# With pdwtags 1.8, --verbose output includes separators like these:
-# /* 93 */
-# /* <0> (null):0 */
-# with the second line omitted for intrinsic types.
-# Whereas with pdwtags 1.3, they look like this:
-# /* <2d2> /usr/include/libio.h:180 */
-# The alternation of the following regexps matches both cases.
-r1 = /\* \d+ \*/
-r2 = /\* <[[:xdigit:]]+> \S+:\d+ \*/
-libs_prefix = remote_|qemu_|lxc_|admin_
-other_prefix = keepalive|vir(Net|LockSpace|LXCMonitor)
-struct_prefix = ($(libs_prefix)|$(other_prefix))
-
-# Depending on configure options, libtool creates one or both of
-# remote/{,.libs/}libvirt_driver_remote_la-remote_protocol.o. We want
-# the newest of the two, in case configure options changed and a stale
-# file is left around from an earlier build.
So I assume the configure option that gives you the path without .libs
is gone?
-# The pdwtags output is completely different when building with clang
-# which causes the comparison against expected output to fail, so skip
-# if using clang as CC.
-PDWTAGS = \
- $(AM_V_GEN)if $(CC) -v 2>&1 | grep -q clang; then \
- echo 'WARNING: skipping pdwtags test with Clang' >&2; \
- exit 0; \
- fi; \
- if (pdwtags --help) > /dev/null 2>&1; then \
- o=`ls -t $(<:.lo=.$(OBJEXT)) \
-$(subst /,/.libs/,$(<:.lo=.$(OBJEXT))) \
- 2>/dev/null | sed -n 1p`; \
And that OBJEXT is always .o
- test -f "$$o" || { echo ".o for $< not found" >&2; exit 1; }; \
- pdwtags --verbose $$o > $(@F)-t1 2> $(@F)-t2; \
- if test ! -s $(@F)-t1 && test -s $(@F)-t2; then \
- rm -rf $(@F)-t?; \
- echo 'WARNING: pdwtags appears broken; skipping the $@ test' >&2;\
- else \
- $(PERL) -0777 -n \
- -e 'foreach my $$p (split m!\n*(?:$(r1)|$(r2))\n!) {' \
- -e ' if ($$p =~ /^(struct|enum) $(struct_prefix)/) {' \
- -e '$$p =~ s!\t*/\*.*?\*/!!sg;' \
- -e '$$p =~ s!\s+\n!\n!sg;' \
- -e '$$p =~ s!\s+$$!!;' \
- -e '$$p =~ s!\t!!g;' \
- -e 'print "$$p\n";' \
- -e '$$n++;' \
- -e ' }' \
- -e '}' \
- -e 'BEGIN {' \
- -e ' print "/* -*- c -*- */\n";' \
- -e '}' \
- -e 'END {' \
- -e ' if ($$n < 1) {' \
- -e 'warn "WARNING: your pdwtags program is too old\n";' \
- -e 'warn "WARNING: skipping the $@ test\n";' \
- -e 'warn "WARNING: install dwarves-1.3 or newer\n";' \
- -e 'exit 8;' \
- -e ' }' \
- -e '}' \
- < $(@F)-t1 > $(@F)-t3; \
- case $$? in 8) rm -f $(@F)-t?; exit 0;; 0) ;; *) exit 1;; esac;\
- diff -u $(@)s $(@F)-t3; st=$$?; rm -f $(@F)-t?; exit $$st; \
- fi; \
- else \
- echo 'WARNING: you lack pdwtags; skipping the $@ test' >&2; \
- echo 'WARNING: install the dwarves
Re: [libvirt] [PATCH 10/09] gnulib: remove mk*temp modules
On Wed, Nov 20, 2019 at 13:07:18 +0100, Ján Tomko wrote: > After commits 4ac47730408eaf91683f6502ec10541f4f711a5c and > ef88698668e4a87c794d70879eeffefb52aa0017, we use the GLib versions > of these functions. > > Remove the corresponding gnulib modules. > > Signed-off-by: Ján Tomko > Reviewed-by: Peter Krempa > --- > With respect to established traditions, I already included Peter's > R-b: > https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=9c68bb4a5c2c8c050e531533e5188a637ba1dd9f Thanks for saving me some work ;) ... I'll push this series shortly. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 13/23] src: rewrite polkit ACL generator in Python
On Mon, Nov 11, 2019 at 02:38:16PM +, Daniel P. Berrangé wrote: As part of an goal to eliminate Perl from libvirt build tools, rewrite the genpolkit.pl tool in Python. This was a straight conversion, manually going line-by-line to change the syntax from Perl to Python. Thus the overall structure of the file and approach is the same. Signed-off-by: Daniel P. Berrangé --- Makefile.am| 1 + scripts/genpolkit.py | 122 + src/access/Makefile.inc.am | 6 +- src/access/genpolkit.pl| 119 4 files changed, 126 insertions(+), 122 deletions(-) create mode 100755 scripts/genpolkit.py delete mode 100755 src/access/genpolkit.pl Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 9/9] bootstrap.conf: drop c-strcasestr gnulib module
On Wed, Nov 20, 2019 at 12:42:09PM +0100, Peter Krempa wrote: From: Pavel Hrdina Last usage was removed by commit <41f6198e231285cc813f8c0687c8ec5c9488> and commit <0f4d31720430b4e3735064cc0d8f88a1a438e154> forgot to drop include. Signed-off-by: Pavel Hrdina Reviewed-by: Peter Krempa Reviewed-by: Fabiano Fidêncio --- bootstrap.conf | 1 - src/qemu/qemu_monitor_json.c | 1 - 2 files changed, 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/09] gnulib: remove mk*temp modules
After commits 4ac47730408eaf91683f6502ec10541f4f711a5c and ef88698668e4a87c794d70879eeffefb52aa0017, we use the GLib versions of these functions. Remove the corresponding gnulib modules. Signed-off-by: Ján Tomko Reviewed-by: Peter Krempa --- With respect to established traditions, I already included Peter's R-b: https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=9c68bb4a5c2c8c050e531533e5188a637ba1dd9f bootstrap.conf | 3 --- 1 file changed, 3 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 4c784487e2..a3320a28c9 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -53,9 +53,6 @@ listen localeconv manywarnings mgetgroups -mkdtemp -mkostemp -mkostemps net_if netdb nonblocking -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/9] syntax-check: forbid usage of snprintf
On Wed, Nov 20, 2019 at 12:42:06PM +0100, Peter Krempa wrote: From: Pavel Hrdina Signed-off-by: Pavel Hrdina Reviewed-by: Peter Krempa --- build-aux/syntax-check.mk | 8 1 file changed, 8 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/9] syntax-check: update of sprintf rule to mention g_snpritnf
s/g_snpritnf/g_snprintf/ On Wed, Nov 20, 2019 at 12:42:05PM +0100, Peter Krempa wrote: From: Pavel Hrdina Signed-off-by: Pavel Hrdina Reviewed-by: Peter Krempa --- build-aux/syntax-check.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) with that fixed: Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/9] gnulib: remove 'areadlink' module
On Wed, Nov 20, 2019 at 12:42:01PM +0100, Peter Krempa wrote: Signed-off-by: Peter Krempa --- bootstrap.conf | 1 - 1 file changed, 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/9] bootstrap.conf: remove usage of snprintf gnulib module
On Wed, Nov 20, 2019 at 12:42:04PM +0100, Peter Krempa wrote: From: Pavel Hrdina Signed-off-by: Pavel Hrdina Reviewed-by: Peter Krempa --- bootstrap.conf | 1 - 1 file changed, 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/9] bootstrap: remove regex module
From: Ján Tomko Now that we use GRegex everywhere, there is no need for this module. Signed-off-by: Ján Tomko Reviewed-by: Peter Krempa --- bootstrap.conf | 1 - po/POTFILES.in | 1 - 2 files changed, 2 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 4ba3ebfdd5..f13fcae11a 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -66,7 +66,6 @@ poll posix-shell pthread_sigmask recv -regex send setenv setsockopt diff --git a/po/POTFILES.in b/po/POTFILES.in index f93fb9694d..debb51cd70 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -6,7 +6,6 @@ @BUILDDIR@/src/remote/remote_client_bodies.h @BUILDDIR@/src/remote/remote_daemon_dispatch_stubs.h @SRCDIR@/gnulib/lib/gai_strerror.c -@SRCDIR@/gnulib/lib/regcomp.c @SRCDIR@/src/access/viraccessdriverpolkit.c @SRCDIR@/src/access/viraccessmanager.c @SRCDIR@/src/admin/admin_server.c -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/9] syntax-check: forbid usage of snprintf
From: Pavel Hrdina Signed-off-by: Pavel Hrdina Reviewed-by: Peter Krempa --- 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 6b62e20ed4..f1e976ec76 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -482,6 +482,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 2/9] gnulib: remove use of 'byteswap' module
From: Ján Tomko Signed-off-by: Ján Tomko Reviewed-by: Peter Krempa --- bootstrap.conf | 1 - 1 file changed, 1 deletion(-) diff --git a/bootstrap.conf b/bootstrap.conf index 6156fa8499..1d80640c0a 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -20,7 +20,6 @@ gnulib_modules=' accept bind -byteswap c-ctype c-strcase c-strcasestr -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/9] syntax-check: update of sprintf rule to mention g_snpritnf
From: Pavel Hrdina Signed-off-by: Pavel Hrdina Reviewed-by: Peter Krempa --- 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 ba941746a1..6b62e20ed4 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -475,11 +475,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 3/9] gnulib: remove use of 'vsnprintf' module
From: Ján Tomko Signed-off-by: Ján Tomko Reviewed-by: Peter Krempa --- bootstrap.conf | 1 - 1 file changed, 1 deletion(-) diff --git a/bootstrap.conf b/bootstrap.conf index 1d80640c0a..c93daf5a8d 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -88,7 +88,6 @@ ttyname_r uname unsetenv verify -vsnprintf waitpid warnings wcwidth -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 9/9] bootstrap.conf: drop c-strcasestr gnulib module
From: Pavel Hrdina Last usage was removed by commit <41f6198e231285cc813f8c0687c8ec5c9488> and commit <0f4d31720430b4e3735064cc0d8f88a1a438e154> forgot to drop include. Signed-off-by: Pavel Hrdina Reviewed-by: Peter Krempa Reviewed-by: Fabiano Fidêncio --- bootstrap.conf | 1 - src/qemu/qemu_monitor_json.c | 1 - 2 files changed, 2 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index e519c69d30..55b48da0e8 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -22,7 +22,6 @@ accept bind c-ctype c-strcase -c-strcasestr canonicalize-lgpl chown clock-time diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 10f6a4cadc..fb662cb18b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -40,7 +40,6 @@ #include "virprobe.h" #include "virstring.h" #include "cpu/cpu_x86.h" -#include "c-strcasestr.h" #include "virenum.h" #ifdef WITH_DTRACE_PROBES -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/9] gnulib: remove 'areadlink' module
Signed-off-by: Peter Krempa --- bootstrap.conf | 1 - 1 file changed, 1 deletion(-) diff --git a/bootstrap.conf b/bootstrap.conf index 4c784487e2..6156fa8499 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -19,7 +19,6 @@ # gnulib modules used by this package. gnulib_modules=' accept -areadlink bind byteswap c-ctype -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/9] Batch removal of gnulib modules
This is a series of patches collecting gnulib module removals. Patches from Jano and Pavel have alreadny my R-bs but at least one of my patches does not. Jano, I couldn't find your patch for removal of the mkostemp functions so please provide it. I'll push it later today once I get Jano's patch so that we have only one point of disruption. Ján Tomko (3): gnulib: remove use of 'byteswap' module gnulib: remove use of 'vsnprintf' module bootstrap: remove regex module Pavel Hrdina (4): 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: drop c-strcasestr gnulib module Peter Krempa (2): gnulib: remove 'areadlink' module gnulib: Remove use of 'strsep' module bootstrap.conf | 7 --- build-aux/syntax-check.mk| 12 ++-- po/POTFILES.in | 1 - src/qemu/qemu_monitor_json.c | 1 - 4 files changed, 10 insertions(+), 11 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
