[libvirt] [PATCH v8 00/21] Incremental backups
by Eric Blake
The API is fairly stable, but there are still probably code cleanups
worth adding to the qemu driver implementation. I've also seen a
desire on the list to support all APIs in the test driver, so I'll
try and whip that together for my next round of patches.
If we are confident that the API is worth having in 5.3 (especially
since the earlier part of this series already has positive reviews),
then it's probably worth committing that part of the series now even
while I still address review comments on the later patches.
I've pushed a tag backup-v8 to both my libvirt.git and
libvirt-python.git repos to match:
https://repo.or.cz/libvirt/ericb.git/shortlog/refs/tags/backup-v8
https://repo.or.cz/libvirt-python/ericb.git/shortlog/refs/tags/backup-v8
Here's hoping we're happy enough with the API, including the fact that
rudimentary operation of pull mode backups work with qemu 4.0-rc1, for
this to make it into the 5.2 release.
Notable diffs from v7:
- address several review comments (although I didn't get to every
comment later in the series)
- actually support job id of 0 (it was documented but not working in v7)
- add support for Unix socket pull mode backups
- improve checkpoint XML handling (more tests, separate postparse actions
out from initial parse)
- rebase to master
001/21:[0044] [FC] 'backup: Document new XML for checkpoints'
002/21:[----] [--] 'backup: Document new XML for backups'
003/21:[0006] [FC] 'backup: Introduce virDomainCheckpoint APIs'
004/21:[0002] [FC] 'backup: Introduce virDomainBackup APIs'
005/21:[----] [--] 'backup: Document nuances between different state capture APIs'
006/21:[0304] [FC] 'backup: Parse and output checkpoint XML'
007/21:[0007] [FC] 'backup: Allow for lists of checkpoint objects'
008/21:[0004] [FC] 'backup: Add new domain:checkpoint access control'
009/21:[0043] [FC] 'backup: Implement backup APIs for remote driver'
010/21:[down] 'conf: Add parameter to virDomainDiskSourceFormat'
011/21:[0019] [FC] 'backup: Parse and output backup XML'
012/21:[0006] [FC] 'backup: Implement virsh support for checkpoints'
013/21:[0002] [FC] 'backup: Implement virsh support for backup'
014/21:[down] 'backup: Prepare for Unix sockets in QMP nbd-server-start'
015/21:[0008] [FC] 'backup: Add new qemu monitor interactions'
016/21:[0022] [FC] 'backup: qemu: Implement metadata tracking for checkpoint APIs'
017/21:[0004] [FC] 'backup: Wire up qemu checkpoint commands over QMP'
018/21:[0019] [FC] 'backup: qemu: Implement framework for backup job APIs'
019/21:[0042] [FC] 'backup: Wire up qemu full pull backup commands over QMP'
020/21:[----] [-C] 'backup: qemu: Wire up qemu full push backup commands over QMP'
021/21:[0003] [FC] 'backup: implement qemu incremental pull backup'
Eric Blake (21):
backup: Document new XML for checkpoints
backup: Document new XML for backups
backup: Introduce virDomainCheckpoint APIs
backup: Introduce virDomainBackup APIs
backup: Document nuances between different state capture APIs
backup: Parse and output checkpoint XML
backup: Allow for lists of checkpoint objects
backup: Add new domain:checkpoint access control
backup: Implement backup APIs for remote driver
conf: Add parameter to virDomainDiskSourceFormat
backup: Parse and output backup XML
backup: Implement virsh support for checkpoints
backup: Implement virsh support for backup
backup: Prepare for Unix sockets in QMP nbd-server-start
backup: Add new qemu monitor interactions
backup: qemu: Implement metadata tracking for checkpoint APIs
backup: Wire up qemu checkpoint commands over QMP
backup: qemu: Implement framework for backup job APIs
backup: Wire up qemu full pull backup commands over QMP
backup: qemu: Wire up qemu full push backup commands over QMP
backup: implement qemu incremental pull backup
include/libvirt/libvirt-domain-checkpoint.h | 161 ++
include/libvirt/libvirt-domain-snapshot.h | 8 +-
include/libvirt/libvirt-domain.h | 47 +-
include/libvirt/libvirt.h | 5 +-
src/access/viraccessperm.h | 6 +
src/conf/backup_conf.h | 97 +
src/conf/checkpoint_conf.h | 103 ++
src/conf/domain_conf.h | 3 +
src/conf/virconftypes.h | 12 +
src/conf/virdomaincheckpointobjlist.h | 74 +
src/conf/virdomainmomentobjlist.h | 7 +-
src/conf/virdomainobjlist.h | 7 +-
src/driver-hypervisor.h | 79 +
src/qemu/qemu_block.h | 3 +
src/qemu/qemu_blockjob.h | 1 +
src/qemu/qemu_capabilities.h | 2 +
src/qemu/qemu_conf.h | 2 +
src/qemu/qemu_domain.h | 19 +
src/qemu/qemu_monitor.h | 27 +-
src/qemu/qemu_monitor_json.h | 22 +-
tools/virsh-checkpoint.h | 29 +
tools/virsh-completer.h | 4 +
tools/virsh-util.h | 3 +
tools/virsh.h | 1 +
docs/Makefile.am | 3 +
docs/apibuild.py | 2 +
docs/docs.html.in | 10 +-
docs/domainstatecapture.html.in | 315 ++++
docs/format.html.in | 2 +
docs/formatbackup.html.in | 184 ++
docs/formatcheckpoint.html.in | 204 +++
docs/formatsnapshot.html.in | 2 +
docs/index.html.in | 4 +-
docs/schemas/domainbackup.rng | 219 +++
docs/schemas/domaincheckpoint.rng | 87 +
examples/object-events/event-test.c | 3 +
libvirt.spec.in | 3 +
mingw-libvirt.spec.in | 6 +
po/POTFILES | 3 +
src/Makefile.am | 2 +
src/access/viraccessperm.c | 3 +-
src/conf/Makefile.inc.am | 6 +
src/conf/backup_conf.c | 538 ++++++
src/conf/checkpoint_conf.c | 636 +++++++
src/conf/domain_conf.c | 22 +-
src/conf/snapshot_conf.c | 3 +-
src/conf/virdomaincheckpointobjlist.c | 223 +++
src/conf/virdomainmomentobjlist.c | 40 +-
src/conf/virdomainobjlist.c | 11 +
src/conf/virdomainsnapshotobjlist.c | 2 +-
src/libvirt-domain-checkpoint.c | 750 ++++++++
src/libvirt-domain-snapshot.c | 89 +
src/libvirt-domain.c | 237 ++-
src/libvirt_private.syms | 32 +
src/libvirt_public.syms | 24 +
src/qemu/qemu_block.c | 12 +
src/qemu/qemu_capabilities.c | 4 +
src/qemu/qemu_conf.c | 5 +
src/qemu/qemu_domain.c | 221 ++-
src/qemu/qemu_driver.c | 1577 +++++++++++++++++
src/qemu/qemu_migration.c | 9 +-
src/qemu/qemu_monitor.c | 72 +-
src/qemu/qemu_monitor_json.c | 222 ++-
src/qemu/qemu_process.c | 9 +
src/remote/remote_daemon_dispatch.c | 20 +
src/remote/remote_driver.c | 32 +-
src/remote/remote_protocol.x | 258 ++-
src/remote_protocol-structs | 139 ++
src/rpc/gendispatch.pl | 32 +-
tests/Makefile.am | 13 +-
tests/domainbackupxml2xmlin/backup-pull.xml | 9 +
tests/domainbackupxml2xmlin/backup-push.xml | 9 +
tests/domainbackupxml2xmlin/empty.xml | 1 +
tests/domainbackupxml2xmlout/backup-pull.xml | 9 +
tests/domainbackupxml2xmlout/backup-push.xml | 9 +
tests/domainbackupxml2xmlout/empty.xml | 7 +
tests/domaincheckpointxml2xmlin/empty.xml | 1 +
tests/domaincheckpointxml2xmlin/sample.xml | 7 +
tests/domaincheckpointxml2xmlin/size.xml | 4 +
tests/domaincheckpointxml2xmlout/empty.xml | 7 +
.../internal-active-invalid.xml | 53 +
.../internal-inactive-invalid.xml | 53 +
tests/domaincheckpointxml2xmlout/redefine.xml | 63 +
tests/domaincheckpointxml2xmlout/sample.xml | 12 +
tests/domaincheckpointxml2xmlout/size.xml | 11 +
tests/domaincheckpointxml2xmltest.c | 223 +++
tests/qemublocktest.c | 3 +-
.../caps_4.0.0.riscv32.xml | 2 +
.../caps_4.0.0.riscv64.xml | 2 +
.../caps_4.0.0.x86_64.xml | 2 +
tests/qemumonitorjsontest.c | 32 +-
tests/virschematest.c | 4 +
tests/virstoragetest.c | 2 +-
tools/Makefile.am | 1 +
tools/virsh-checkpoint.c | 1370 ++++++++++++++
tools/virsh-completer.c | 51 +
tools/virsh-domain-monitor.c | 23 +
tools/virsh-domain.c | 268 ++-
tools/virsh-snapshot.c | 37 +-
tools/virsh-util.c | 11 +
tools/virsh.c | 2 +
tools/virsh.pod | 302 +++-
102 files changed, 9502 insertions(+), 100 deletions(-)
create mode 100644 include/libvirt/libvirt-domain-checkpoint.h
create mode 100644 src/conf/backup_conf.h
create mode 100644 src/conf/checkpoint_conf.h
create mode 100644 src/conf/virdomaincheckpointobjlist.h
create mode 100644 tools/virsh-checkpoint.h
create mode 100644 docs/domainstatecapture.html.in
create mode 100644 docs/formatbackup.html.in
create mode 100644 docs/formatcheckpoint.html.in
create mode 100644 docs/schemas/domainbackup.rng
create mode 100644 docs/schemas/domaincheckpoint.rng
create mode 100644 src/conf/backup_conf.c
create mode 100644 src/conf/checkpoint_conf.c
create mode 100644 src/conf/virdomaincheckpointobjlist.c
create mode 100644 src/libvirt-domain-checkpoint.c
create mode 100644 tests/domainbackupxml2xmlin/backup-pull.xml
create mode 100644 tests/domainbackupxml2xmlin/backup-push.xml
create mode 100644 tests/domainbackupxml2xmlin/empty.xml
create mode 100644 tests/domainbackupxml2xmlout/backup-pull.xml
create mode 100644 tests/domainbackupxml2xmlout/backup-push.xml
create mode 100644 tests/domainbackupxml2xmlout/empty.xml
create mode 100644 tests/domaincheckpointxml2xmlin/empty.xml
create mode 100644 tests/domaincheckpointxml2xmlin/sample.xml
create mode 100644 tests/domaincheckpointxml2xmlin/size.xml
create mode 100644 tests/domaincheckpointxml2xmlout/empty.xml
create mode 100644 tests/domaincheckpointxml2xmlout/internal-active-invalid.xml
create mode 100644 tests/domaincheckpointxml2xmlout/internal-inactive-invalid.xml
create mode 100644 tests/domaincheckpointxml2xmlout/redefine.xml
create mode 100644 tests/domaincheckpointxml2xmlout/sample.xml
create mode 100644 tests/domaincheckpointxml2xmlout/size.xml
create mode 100644 tests/domaincheckpointxml2xmltest.c
create mode 100644 tools/virsh-checkpoint.c
--
2.20.1
4 years, 10 months
[libvirt] [PATCHv3 0/5] Implement debugcon chardev
by Ján Tomko
v2: https://www.redhat.com/archives/libvir-list/2019-February/msg00293.html
v3:
* dropped the pointless isa-prefix
* use DO_TEST_CAPS_LATEST
* trimmed the XML
* compiles with gcc
* only make irq optional
* autofill iobase
Ján Tomko (3):
qemu: introduce qemuDomainChrSerialTargetModel
qemu: make irq optional when formatting the ISA address
qemu: autoadd iobase to debugcon chardev
Nikolay Shirokovskiy (2):
conf: add debugcon chardev guest interface
qemu: implement debugcon chardev
docs/formatdomain.html.in | 3 +-
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_conf.c | 5 ++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c | 25 +++++++++++++---
src/qemu/qemu_domain.c | 12 ++++++++
.../isa-serial-debugcon.x86_64-latest.args | 30 +++++++++++++++++++
.../qemuxml2argvdata/isa-serial-debugcon.xml | 21 +++++++++++++
tests/qemuxml2argvtest.c | 1 +
.../isa-serial-debugcon.xml | 30 +++++++++++++++++++
tests/qemuxml2xmltest.c | 2 ++
11 files changed, 126 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuxml2argvdata/isa-serial-debugcon.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/isa-serial-debugcon.xml
create mode 100644 tests/qemuxml2xmloutdata/isa-serial-debugcon.xml
--
2.19.2
4 years, 11 months
Re: [libvirt] [PATCH] storage: escape ipv6 for ceph mon hosts to librados
by Yi Li
> >Hosts for rbd are ceph monitor daemons. These have fixed IP addresses,
> >so they are often referenced by IP rather than hostname for
> >convenience, or to avoid relying on DNS. Using IPv4 addresses as the
> >host name works already, but IPv6 addresses require rbd-specific
>
> If you include the escaping in the XML, does it currently work?
> <host name='[2205::192:168:205:141]' port='6789'/>
Yes, It works.
>
>
> >escaping because the colon is used as an option separator in the
> >string passed to librados.
> >
> >Escape these colons, and enclose the IPv6 address in square brackets
> >so it is distinguished from the port, which is currently mandatory.
> >
> >Signed-off-by: Yi Li <yili(a)winhong.com>
> >---
> > docs/schemas/storagepool.rng | 5 ++++-
> > src/storage/storage_backend_rbd.c | 13 ++++++++++---
> > tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml | 13 +++++++++++++
> > tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml | 16 ++++++++++++++++
> > tests/storagepoolxml2xmltest.c | 1 +
> > 5 files changed, 44 insertions(+), 4 deletions(-)
> > create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml
> > create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml
> >
> >diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
> >index f8c968e..3056563 100644
> >--- a/src/storage/storage_backend_rbd.c
> >+++ b/src/storage/storage_backend_rbd.c
> >@@ -268,9 +268,16 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
> > source->hosts[i].name);
> > } else if (source->hosts[i].name != NULL &&
> > source->hosts[i].port) {
> >- virBufferAsprintf(&mon_host, "%s:%d,",
> >- source->hosts[i].name,
> >- source->hosts[i].port);
> >+ /* assume host containing : is ipv6 */
> >+ if (strchr(source->hosts[i].name, ':')) {
>
> if (virSocketAddrNumericFamily(listenAddress) == AF_INET6)
>
> By using this helper function, we won't try to escape an address that is
> already escaped.
>
> Also, instead of copying the whole virBuffer call twice, it would be
> nicer to assign the format to a temporary variable like we do in qemuMigrationDstPrepare
>
> Jano
Good point. I'd sending a v2.
>
>
> >+ virBufferAsprintf(&mon_host, "[%s]:%d,",
> >+ source->hosts[i].name,
> >+ source->hosts[i].port);
> >+ } else {
> >+ virBufferAsprintf(&mon_host, "%s:%d,",
> >+ source->hosts[i].name,
> >+ source->hosts[i].port);
> >+ }
> > } else {
> > virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > _("received malformed monitor, check the XML definition"));
On Sun, Apr 28, 2019 at 1:10 PM winhong-yili <yili(a)winhong.com> wrote:
>
> >Hosts for rbd are ceph monitor daemons. These have fixed IP addresses,
> >so they are often referenced by IP rather than hostname for
> >convenience, or to avoid relying on DNS. Using IPv4 addresses as the
> >host name works already, but IPv6 addresses require rbd-specific
>
> If you include the escaping in the XML, does it currently work?
> <host name='[2205::192:168:205:141]' port='6789'/>
>
> >escaping because the colon is used as an option separator in the
> >string passed to librados.
> >
> >Escape these colons, and enclose the IPv6 address in square brackets
> >so it is distinguished from the port, which is currently mandatory.
> >
> >Signed-off-by: Yi Li <yili(a)winhong.com>
> >---
> > docs/schemas/storagepool.rng | 5 ++++-
> > src/storage/storage_backend_rbd.c | 13 ++++++++++---
> > tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml | 13 +++++++++++++
> > tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml | 16 ++++++++++++++++
> > tests/storagepoolxml2xmltest.c | 1 +
> > 5 files changed, 44 insertions(+), 4 deletions(-)
> > create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml
> > create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml
> >
> >diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
> >index f8c968e..3056563 100644
> >--- a/src/storage/storage_backend_rbd.c
> >+++ b/src/storage/storage_backend_rbd.c
> >@@ -268,9 +268,16 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
> > source->hosts[i].name);
> > } else if (source->hosts[i].name != NULL &&
> > source->hosts[i].port) {
> >- virBufferAsprintf(&mon_host, "%s:%d,",
> >- source->hosts[i].name,
> >- source->hosts[i].port);
> >+ /* assume host containing : is ipv6 */
> >+ if (strchr(source->hosts[i].name, ':')) {
>
> if (virSocketAddrNumericFamily(listenAddress) == AF_INET6)
>
> By using this helper function, we won't try to escape an address that is
> already escaped.
>
> Also, instead of copying the whole virBuffer call twice, it would be
> nicer to assign the format to a temporary variable like we do in qemuMigrationDstPrepare
>
> Jano
>
> >+ virBufferAsprintf(&mon_host, "[%s]:%d,",
> >+ source->hosts[i].name,
> >+ source->hosts[i].port);
> >+ } else {
> >+ virBufferAsprintf(&mon_host, "%s:%d,",
> >+ source->hosts[i].name,
> >+ source->hosts[i].port);
> >+ }
> > } else {
> > virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > _("received malformed monitor, check the XML definition"));
4 years, 11 months
[libvirt] [PATCH v3 00/15] implement cgroups v2 support
by Pavel Hrdina
In cgroups v2 there is no devices controller, eBPF should be used
instead.
Changes in v3:
- removed workaround for kernel bug [1]
- added documentation how to get the eBPF program
Changes in v2:
- fixed build on bsd and older kernels without cgroup BPF
- cgroup bpf devices code moved to separate file
Documentation for eBPF:
<http://man7.org/linux/man-pages/man2/bpf.2.html>
<https://www.kernel.org/doc/Documentation/networking/filter.txt>
<https://docs.cilium.io/en/v1.3/bpf/>
[1] <https://bugzilla.redhat.com/show_bug.cgi?id=1656432>
Pavel Hrdina (15):
util: introduce virbpf helpers
vircgroup: introduce virCgroupV2DevicesAvailable
vircgroup: introduce virCgroupV2DevicesAttachProg
vircgroup: introduce virCgroupV2DevicesDetectProg
vircgroup: introduce virCgroupV2DevicesCreateProg
vircgroup: introduce virCgroupV2DevicesPrepareProg
vircgroup: introduce virCgroupV2DevicesRemoveProg
vircgroup: introduce virCgroupV2DeviceGetPerms
vircgroup: introduce virCgroupV2DevicesGetKey
vircgroup: introduce virCgroupV2AllowDevice
vircgroup: introduce virCgroupV2DenyDevice
vircgroup: introduce virCgroupV2AllowAllDevices
vircgroup: introduce virCgroupV2DenyAllDevices
vircgroup: workaround devices in hybrid mode
vircgroupmock: mock virCgroupV2DevicesAvailable
configure.ac | 6 +
include/libvirt/virterror.h | 2 +
src/Makefile.am | 2 +
src/libvirt_private.syms | 26 ++
src/util/Makefile.inc.am | 4 +
src/util/virbpf.c | 438 +++++++++++++++++++
src/util/virbpf.h | 259 ++++++++++++
src/util/vircgroup.c | 3 +-
src/util/vircgroupbackend.h | 3 +-
src/util/vircgrouppriv.h | 10 +
src/util/vircgroupv1.c | 9 +-
src/util/vircgroupv2.c | 117 +++++-
src/util/vircgroupv2devices.c | 670 ++++++++++++++++++++++++++++++
src/util/vircgroupv2devices.h | 57 +++
src/util/virerror.c | 2 +
tests/vircgroupdata/hybrid.parsed | 2 +-
tests/vircgroupmock.c | 7 +
tests/vircgrouptest.c | 4 +-
18 files changed, 1613 insertions(+), 8 deletions(-)
create mode 100644 src/util/virbpf.c
create mode 100644 src/util/virbpf.h
create mode 100644 src/util/vircgroupv2devices.c
create mode 100644 src/util/vircgroupv2devices.h
--
2.20.1
4 years, 11 months
[libvirt] QMP; unsigned 64-bit ints; JSON standards compliance
by Daniel P. Berrangé
The QEMU QMP service is based on JSON which is nice because that is a
widely supported "standard" data format.....
....except QEMU's implementation (and indeed most impls) are not strictly
standards compliant.
Specifically the problem is around representing 64-bit integers, whether
signed or unsigned.
The JSON standard declares that largest integer is 2^53-1 and the
likewise the smallest is -(2^53-1):
http://www.ecma-international.org/ecma-262/6.0/index.html#sec-number.max_...
A crazy limit inherited from its javascript origins IIUC.
QEMU, and indeed many applications, want to handle 64-bit integers.
The C JSON library impls have traditionally mapped integers to the
data type 'long long int' which gives a min/max of -(2^63) / 2^63-1.
QEMU however /really/ needs 64-bit unsigned integers, ie a max 2^64-1.
Libvirt has historically used the YAJL library which uses 'long long int'
and thus can't officially go beyond 2^63-1 values. Fortunately it lets
libvirt get at the raw json string, so libvirt can re-parse the value
to get an 'unsigned long long'.
We recently tried to switch to Jansson because YAJL has a dead upstream
for many years and countless unanswered bugs & patches. Unfortunately we
forgot about this need for 2^64-1 max, and Jansson also uses 'long long int'
and raises a fatal parse error for unsigned 64-bit values above 2^63-1. It
also provides no backdoor for libvirt todo its own integer parsing. Thus
we had to abort our switch to jansson as it broke parsing QEMU's JSON:
https://bugzilla.redhat.com/show_bug.cgi?id=1614569
Other JSON libraries we've investigated have similar problems. I imagine
the same may well be true of non-C based JOSN impls, though I've not
investigated in any detail.
Essentially libvirt is stuck with either using the dead YAJL library
forever, or writing its own JSON parser (most likely copying QEMU's
JSON code into libvirt's git).
This feels like a very unappealing situation to be in as not being
able to use a JSON library of our choice is loosing one of the key
benefits of using a standard data format.
Thus I'd like to see a solution to this to allow QMP to be reliably
consumed by any JSON library that exists.
I can think of some options:
1. Encode unsigned 64-bit integers as signed 64-bit integers.
This follows the example that most C libraries map JSON ints
to 'long long int'. This is still relying on undefined
behaviour as apps don't need to support > 2^53-1.
Apps would need to cast back to 'unsigned long long' for
those QMP fields they know are supposed to be unsigned.
2. Encode all 64-bit integers as a pair of 32-bit integers.
This is fully compliant with the JSON spec as each half
is fully within the declared limits. App has to split or
assemble the 2 pieces from/to a signed/unsigned 64-bit
int as needed.
3. Encode all 64-bit integers as strings
The application has todo all parsing/formatting client
side.
None of these changes are backwards compatible, so I doubt we could make
the change transparently in QMP. Instead we would have to have a
QMP greeting message capability where the client can request enablement
of the enhanced integer handling.
Any of the three options above would likely work for libvirt, but I
would have a slight preference for either 2 or 3, so that we become
100% standards compliant.
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 :|
4 years, 11 months
[libvirt] [PATCH v2 for 5.3.0] storage_backend_rbd: Ignore rbd_diff_iterate error() if needed
by Michal Privoznik
When cloning an RBD volume we try to find a snapshot which is
not different to the image we're trying to clone. This boils down
to calling rbd_diff_iterate() or rbd_diff_iterate2() on systems
with newer ceph. These two are passed a callback -
virStorageBackendRBDIterateCb() which sets @diff to 1 and returns
-1. The idea is to stop iterating as soon as possible (i.e. we're
iterating over a snapshot with some deltas). Unfortunately,
returning a negative value from the callback means that iterate()
function fails and thus we report an error:
virsh # vol-clone --pool rbd_image_root coreos_2023 coreos00.disk
error: Failed to clone vol from coreos_2023
error: failed to iterate RBD snapshot coreos_2023@base: Operation not
permitted
Therefore, report an error if and only if it doesn't originates
in the callback.
Reported on libvirt-users list:
https://www.redhat.com/archives/libvirt-users/2019-April/msg00060.html
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
diff to v1:
- Ignore error as suggested by Jason as it allows us to exit iteration
early.
src/storage/storage_backend_rbd.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index f8c968e682..69f4fa2117 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -1101,7 +1101,9 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image,
virStorageBackendRBDIterateCb, (void *)&diff);
#endif
- if (r < 0) {
+ /* We care for errors only if the callback wasn't called at all. If it
+ * was, rbd_diff_iterate() returns an error which we have to ignore. */
+ if (r < 0 && diff == 0) {
virReportSystemError(-r, _("failed to iterate RBD snapshot %s@%s"),
imgname, snaps[i].name);
goto cleanup;
--
2.21.0
5 years
[libvirt] [PATCH v1] qemu: migration: Convert to virErrorRestore/virErrorPreserveLast
by Syed Humaid
Replaced usages of virSaveLastError and virSetError/virFreeError with
virErrorPreserveLast and virErrorRestore respectively.
Signed-off-by: Syed Humaid <syedhumaidbinharoon(a)gmail.com>
---
src/qemu/qemu_migration.c | 75 +++++++++++++--------------------------
1 file changed, 24 insertions(+), 51 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index e3ad4e52a7..b1fe79aed9 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -722,7 +722,7 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriverPtr driver,
if (rv != 0) {
if (rv < 0) {
if (!err)
- err = virSaveLastError();
+ virErrorPreserveLast(&err);
failed = true;
}
qemuBlockJobSyncEnd(vm, job, asyncJob);
@@ -747,7 +747,7 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriverPtr driver,
}
if (failed && !err)
- err = virSaveLastError();
+ virErrorPreserveLast(&err);
if (virDomainObjWait(vm) < 0)
goto cleanup;
@@ -769,10 +769,7 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriverPtr driver,
ret = failed ? -1 : 0;
cleanup:
- if (err) {
- virSetError(err);
- virFreeError(err);
- }
+ virErrorRestore(&err);
return ret;
}
@@ -3189,16 +3186,13 @@ static void qemuMigrationSrcIOFunc(void *arg)
return;
abrt:
- err = virSaveLastError();
+ virErrorPreserveLast(&err);
if (err && err->code == VIR_ERR_OK) {
virFreeError(err);
err = NULL;
}
virStreamAbort(data->st);
- if (err) {
- virSetError(err);
- virFreeError(err);
- }
+ virErrorRestore(&err);
error:
/* Let the source qemu know that the transfer cant continue anymore.
@@ -3685,15 +3679,12 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver,
if (events)
priv->signalIOError = false;
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
return ret;
error:
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
if (virDomainObjIsActive(vm)) {
if (cancel &&
@@ -3948,7 +3939,7 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriverPtr driver,
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("domainMigratePrepare2 did not set uri"));
cancelled = true;
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
goto finish;
}
@@ -3971,7 +3962,7 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriverPtr driver,
/* Perform failed. Make sure Finish doesn't overwrite the error */
if (ret < 0)
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
/* If Perform returns < 0, then we need to cancel the VM
* startup on the destination
@@ -4004,10 +3995,7 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriverPtr driver,
virObjectUnref(st);
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
VIR_FREE(uri_out);
VIR_FREE(cookie);
@@ -4181,13 +4169,13 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver,
if (useParams &&
virTypedParamsReplaceString(¶ms, &nparams,
VIR_MIGRATE_PARAM_URI, uri_out) < 0) {
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
goto finish;
}
} else if (!uri && !(flags & VIR_MIGRATE_TUNNELLED)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("domainMigratePrepare3 did not set uri"));
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
goto finish;
}
@@ -4220,7 +4208,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver,
/* Perform failed. Make sure Finish doesn't overwrite the error */
if (ret < 0) {
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
} else {
qemuMigrationJobSetPhase(driver, vm,
QEMU_MIGRATION_PHASE_PERFORM3_DONE);
@@ -4312,7 +4300,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver,
* one we need to preserve it in case confirm3 overwrites
*/
if (!orig_err)
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
/*
* If cancelled, then src VM will be restarted, else
@@ -4344,10 +4332,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver,
virObjectUnref(st);
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
VIR_FREE(uri_out);
VIR_FREE(cookiein);
VIR_FREE(cookieout);
@@ -4523,15 +4508,12 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriverPtr driver,
}
cleanup:
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
qemuDomainObjEnterRemote(vm);
virConnectUnregisterCloseCallback(dconn, qemuMigrationSrcConnectionClosed);
virObjectUnref(dconn);
ignore_value(qemuDomainObjExitRemote(vm, false));
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
virObjectUnref(cfg);
return ret;
}
@@ -4619,7 +4601,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver,
endjob:
if (ret < 0)
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
/* v2 proto has no confirm phase so we need to reset migration parameters
* here
@@ -4639,10 +4621,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver,
qemuDomainRemoveInactiveJob(driver, vm);
}
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
cleanup:
virObjectEventStateQueue(driver->domainEventState, event);
@@ -5052,7 +5031,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
/* Need to save the current error, in case shutting
* down the process overwrites it
*/
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
/*
* In v3 protocol, the source VM is still available to
@@ -5181,10 +5160,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
VIR_FREE(priv->origname);
virDomainObjEndAPI(&vm);
qemuMigrationCookieFree(mig);
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
virObjectUnref(cfg);
/* Set a special error if Finish is expected to return NULL as a result of
@@ -5289,7 +5265,7 @@ qemuMigrationSrcToFile(virQEMUDriverPtr driver, virDomainObjPtr vm,
if (rc < 0) {
if (rc == -2) {
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
virCommandAbort(cmd);
if (virDomainObjIsActive(vm) &&
qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) {
@@ -5308,7 +5284,7 @@ qemuMigrationSrcToFile(virQEMUDriverPtr driver, virDomainObjPtr vm,
cleanup:
if (ret < 0 && !orig_err)
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
/* Restore max migration bandwidth */
if (virDomainObjIsActive(vm) &&
@@ -5326,10 +5302,7 @@ qemuMigrationSrcToFile(virQEMUDriverPtr driver, virDomainObjPtr vm,
virCommandFree(cmd);
}
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
return ret;
}
--
2.20.1
5 years
[libvirt] [PATCH v1] qemu: Convert to virErrorPreserveLast/virErrorRestore
by Syed Humaid
Replaced usage of virSaveLastError and virSetError/virFreeError with
virErrorPreserveLast and virErrorRestore respectively.
Signed-off-by: Syed Humaid <syedhumaidbinharoon(a)gmail.com>
---
src/qemu/qemu_cgroup.c | 7 ++-----
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain.c | 2 +-
src/qemu/qemu_driver.c | 28 ++++++++--------------------
4 files changed, 12 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index ca76c4fdfa..c8d9ae4808 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -1123,10 +1123,7 @@ qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
if (period) {
virErrorPtr saved = virSaveLastError();
ignore_value(virCgroupSetCpuCfsPeriod(cgroup, old_period));
- if (saved) {
- virSetError(saved);
- virFreeError(saved);
- }
+ virErrorRestore(&saved);
}
return -1;
@@ -1334,7 +1331,7 @@ qemuCgroupEmulatorAllNodesRestore(qemuCgroupEmulatorAllNodesDataPtr data)
if (!data)
return;
- err = virSaveLastError();
+ virErrorPreserveLast(&err);
virCgroupSetCpusetMems(data->emulatorCgroup, data->emulatorMemMask);
virSetError(err);
virFreeError(err);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1425d97b1e..9772e4f9af 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9119,7 +9119,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver,
error:
/* free up any resources in the network driver
* but don't overwrite the original error */
- originalError = virSaveLastError();
+ virErrorPreserveLast(&originalError);
for (i = 0; last_good_net != -1 && i <= last_good_net; i++)
virDomainConfNWFilterTeardown(def->nets[i]);
virSetError(originalError);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 960aaff3c7..dff1d7b1a5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8042,7 +8042,7 @@ void qemuDomainObjTaint(virQEMUDriverPtr driver,
/* We don't care about errors logging taint info, so
* preserve original error, and clear any error that
* is raised */
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
if (!(timestamp = virTimeStringNow()))
goto cleanup;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f48d9256e4..51b16f5aa1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6844,7 +6844,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
* must manually kill it and ignore any error related to
* the process
*/
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
VIR_FORCE_CLOSE(intermediatefd);
VIR_FORCE_CLOSE(*fd);
}
@@ -6855,10 +6855,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
}
VIR_DEBUG("Decompression binary stderr: %s", NULLSTR(errbuf));
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
}
VIR_FORCE_CLOSE(intermediatefd);
@@ -14398,7 +14395,7 @@ qemuDomainSnapshotFSThaw(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
agent = qemuDomainObjEnterAgent(vm);
if (!report)
- err = virSaveLastError();
+ virErrorPreserveLast(&err);
thawed = qemuAgentFSThaw(agent);
if (!report)
virSetError(err);
@@ -15308,7 +15305,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
error:
if (ret < 0) {
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
for (i = 0; i < snapdef->ndisks; i++) {
if (!diskdata[i].src)
continue;
@@ -15351,10 +15348,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
virJSONValueFree(actions);
virObjectUnref(cfg);
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
return ret;
}
@@ -17773,7 +17767,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
if (qemuDomainObjExitMonitor(driver, vm) < 0)
ret = -1;
if (ret < 0) {
- monitor_error = virSaveLastError();
+ virErrorPreserveLast(&monitor_error);
qemuDomainDiskChainElementRevoke(driver, vm, mirror);
goto endjob;
}
@@ -17795,10 +17789,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
VIR_WARN("%s", _("unable to remove just-created copy target"));
virStorageFileDeinit(mirror);
qemuDomainObjEndJob(driver, vm);
- if (monitor_error) {
- virSetError(monitor_error);
- virFreeError(monitor_error);
- }
+ virErrorRestore(&monitor_error);
qemuBlockJobStartupFinalize(job);
cleanup:
@@ -18196,10 +18187,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
if (top_parent && top_parent != disk->src)
qemuDomainDiskChainElementPrepare(driver, vm, top_parent, true, false);
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
}
qemuBlockJobStartupFinalize(job);
qemuDomainObjEndJob(driver, vm);
--
2.20.1
5 years
[libvirt] [PATCH v2] qemu: Add entry for balloon stats stat-htlb-pgalloc and stat-htlb-pgfail
by Han Han
Qemu added reporting of virtio balloon new statistics stat-htlb-pgalloc and
stat-htlb-pgfail since qemu-3.0 commit b7b12644297. The value of
stat-htlb-pgalloc represents the number of successful hugetlb page allocations
while stat-htlb-pgfail represents the number of failed ones. Add this
statistics reporting to libvirt.
To enable this feature for vm, guest kenel >= 4.17 is required because
the exporting hugetlb page allocation for virtio balloon is introduced
since 6c64fe7f.
Signed-off-by: Han Han <hhan(a)redhat.com>
---
include/libvirt/libvirt-domain.h | 14 +++++++++++++-
src/libvirt-domain.c | 5 ++++-
src/qemu/qemu_driver.c | 2 ++
src/qemu/qemu_monitor_json.c | 5 +++++
tools/virsh-domain-monitor.c | 4 ++++
tools/virsh.pod | 2 ++
6 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 7d36820b5a..192d25c1db 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -636,11 +636,23 @@ typedef enum {
*/
VIR_DOMAIN_MEMORY_STAT_DISK_CACHES = 10,
+ /*
+ * The amount of successful hugetlb(Huge Page Tables) allocations via
+ * virtio balloon.
+ */
+ VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGALLOC = 11,
+
+ /*
+ * The amount of failed hugetlb(Huge Page Tables) allocations via
+ * virtio balloon.
+ */
+ VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGFAIL = 12,
+
/*
* The number of statistics supported by this version of the interface.
* To add new statistics, add them to the enum and increase this value.
*/
- VIR_DOMAIN_MEMORY_STAT_NR = 11,
+ VIR_DOMAIN_MEMORY_STAT_NR = 13,
# ifdef VIR_ENUM_SENTINELS
VIR_DOMAIN_MEMORY_STAT_LAST = VIR_DOMAIN_MEMORY_STAT_NR
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 80284a99f0..a95690e8a4 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -5734,7 +5734,10 @@ virDomainGetInterfaceParameters(virDomainPtr domain,
* VIR_DOMAIN_MEMORY_STAT_DISK_CACHES
* Memory that can be reclaimed without additional I/O, typically disk
* caches (in kb).
- *
+ * VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGALLOC
+ * The amount of successful hugetlb(Huge Page Tables) allocations
+ * VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGFAIL
+ * The amount of failed hugetlb(Huge Page Tables) allocations
* Returns: The number of stats provided or -1 in case of failure.
*/
int
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f48d9256e4..2d849beac8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20245,6 +20245,8 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver,
STORE_MEM_RECORD(LAST_UPDATE, "last-update")
STORE_MEM_RECORD(USABLE, "usable")
STORE_MEM_RECORD(DISK_CACHES, "disk_caches")
+ STORE_MEM_RECORD(HUGETLB_PGALLOC, "hugetlb_pgalloc")
+ STORE_MEM_RECORD(HUGETLB_PGFAIL, "hugetlb_pgfail")
}
#undef STORE_MEM_RECORD
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 908967f46c..5e3df5e759 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2150,6 +2150,11 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE, 1);
GET_BALLOON_STATS(statsdata, "stat-disk-caches",
VIR_DOMAIN_MEMORY_STAT_DISK_CACHES, 1024);
+ GET_BALLOON_STATS(statsdata, "stat-htlb-pgalloc",
+ VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGALLOC, 1);
+ GET_BALLOON_STATS(statsdata, "stat-htlb-pgfail",
+ VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGFAIL, 1);
+
ret = got;
cleanup:
virJSONValueFree(cmd);
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index d87475f6f6..0092a9786e 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -376,6 +376,10 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "last_update %llu\n", stats[i].val);
if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_DISK_CACHES)
vshPrint(ctl, "disk_caches %llu\n", stats[i].val);
+ if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGALLOC)
+ vshPrint(ctl, "hugetlb_pgalloc %llu\n", stats[i].val);
+ if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGFAIL)
+ vshPrint(ctl, "hugetlb_pgfail %llu\n", stats[i].val);
}
ret = true;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index afc1684db0..ef27c527d6 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -938,6 +938,8 @@ without causing host swapping (in KiB)
last-update - Timestamp of the last update of statistics (in seconds)
disk_caches - The amount of memory that can be reclaimed without
additional I/O, typically disk caches (in KiB)
+ hugetlb_pgalloc - The number of successful huge page table allocations
+ hugetlb_pgfail - The number of failed huge page table allocations
For QEMU/KVM with a memory balloon, setting the optional I<--period> to a
value larger than 0 in seconds will allow the balloon driver to return
--
2.20.1
5 years
[libvirt] [PATCH] lib: Avoid double free when passing FDs with virCommandPassFD()
by Michal Privoznik
If an FD is passed into a child using:
virCommandPassFD(cmd, fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
then the parent should refrain from touching @fd thereafter. This
is even documented in virCommandPassFD() comment. The reason is
that either at virCommandRun()/virCommandRunAsync() or
virCommandFree() time the @fd will be closed. Closing it earlier,
e.g. right after virCommandPassFD() call might result in
undesired results. Another thread might open a file and receive
the same FD which is then unexpectedly closed by virCommandFree()
or virCommandRun().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 10 ++++++----
src/util/virpolkit.c | 1 +
tests/commandtest.c | 2 +-
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bf1fb539b1..92bd1524db 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8978,17 +8978,19 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
if (qemuSecuritySetTapFDLabel(driver->securityManager,
def, tapfd[i]) < 0)
goto cleanup;
- virCommandPassFD(cmd, tapfd[i],
- VIR_COMMAND_PASS_FD_CLOSE_PARENT);
if (virAsprintf(&tapfdName[i], "%d", tapfd[i]) < 0)
goto cleanup;
+ virCommandPassFD(cmd, tapfd[i],
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+ tapfd[i] = -1;
}
for (i = 0; i < vhostfdSize; i++) {
- virCommandPassFD(cmd, vhostfd[i],
- VIR_COMMAND_PASS_FD_CLOSE_PARENT);
if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0)
goto cleanup;
+ virCommandPassFD(cmd, vhostfd[i],
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+ vhostfd[i] = -1;
}
if (chardev)
diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c
index 25eaad2c63..634b46e82b 100644
--- a/src/util/virpolkit.c
+++ b/src/util/virpolkit.c
@@ -187,6 +187,7 @@ virPolkitAgentCreate(void)
virCommandSetOutputFD(agent->cmd, &outfd);
virCommandSetErrorFD(agent->cmd, &errfd);
virCommandPassFD(agent->cmd, pipe_fd[1], VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+ pipe_fd[1] = -1;
if (virCommandRunAsync(agent->cmd, NULL) < 0)
goto error;
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 816a70860f..146cc4c1bf 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -1024,6 +1024,7 @@ static int test24(const void *unused ATTRIBUTE_UNUSED)
virCommandDaemonize(cmd);
virCommandPassFD(cmd, newfd2, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
virCommandPassFD(cmd, newfd3, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+ newfd2 = newfd3 = -1;
virCommandPassListenFDs(cmd);
if (virCommandRun(cmd, NULL) < 0) {
@@ -1053,7 +1054,6 @@ static int test24(const void *unused ATTRIBUTE_UNUSED)
VIR_FREE(prefix);
virCommandFree(cmd);
VIR_FORCE_CLOSE(newfd1);
- /* coverity[double_close] */
VIR_FORCE_CLOSE(newfd2);
VIR_FORCE_CLOSE(newfd3);
return ret;
--
2.21.0
5 years