]> xenbits.xensource.com Git - libvirt.git/log
libvirt.git
6 years agosecurity: aa-helper: generate more rules for gl devices
Christian Ehrhardt [Tue, 12 Feb 2019 10:12:52 +0000 (11:12 +0100)]
security: aa-helper: generate more rules for gl devices

Change fb01e1a44 "virt-aa-helper: generate rules for gl enabled
graphics devices" implemented the detection for gl enabled
devices in virt-aa-helper. But further testing showed
that it will need much more access for the full gl stack
to work.

Upstream apparmor just recently split those things out and now
has two related abstractions at
https://gitlab.com/apparmor/apparmor/blob/master:
- dri-common at /profiles/apparmor.d/abstractions/dri-common
- mesa: at /profiles/apparmor.d/abstractions/mesa

If would be great to just include that for the majority of
rules, but they are not yet in any distribution so we need
to add rules inspired by them based on the testing that we
can do.

Furthermore qemu with opengl will also probe the backing device
of the rendernode for attributes which should be safe as
read-only wildcard rules.

Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1815452
Acked-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
6 years agosecurity: aa-helper: allow virt-aa-helper to read /dev/dri
Christian Ehrhardt [Tue, 12 Feb 2019 09:33:23 +0000 (10:33 +0100)]
security: aa-helper: allow virt-aa-helper to read /dev/dri

Change fb01e1a44 "virt-aa-helper: generate rules for gl enabled
graphics devices" implemented the detection for gl enabled
devices in virt-aa-helper. But it will in certain cases e.g. if
no rendernode was explicitly specified need to read /dev/dri
which it currently isn't allowed.

Add a rule to the apparmor profile of virt-aa-helper itself to
be able to do that.

Acked-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
6 years agobhyve: add bhyveDomainDefNeedsISAController helper
Roman Bogorodskiy [Sun, 17 Feb 2019 07:27:28 +0000 (11:27 +0400)]
bhyve: add bhyveDomainDefNeedsISAController helper

Add a bhyveDomainDefNeedsISAController() helper function
which by domain configuration determines whether LPC controller is
required or not.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agonews: document bhyve msrs feature
Roman Bogorodskiy [Fri, 25 Jan 2019 17:43:38 +0000 (21:43 +0400)]
news: document bhyve msrs feature

Describe bhyve's ignoring unknown MSRs access feature
introduced by commit e9528f41c6.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
6 years agobhyve: implement ignore unknown MSRs feature
Roman Bogorodskiy [Fri, 25 Jan 2019 15:27:58 +0000 (19:27 +0400)]
bhyve: implement ignore unknown MSRs feature

Implement the MSRs ignore unknown reads and writes feature
that's specified using:

  <features>
    ...
    <msrs unknown='ignore'>
    ...
  </features>

in the domain XML.

In bhyve, it's just passing '-w' command line argument to the bhyve(8)
executable.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
6 years agoconf: introduce 'msrs' feature
Roman Bogorodskiy [Thu, 24 Jan 2019 03:23:31 +0000 (07:23 +0400)]
conf: introduce 'msrs' feature

Introduce the 'msrs' feature element that controls Model Specific
Registers related behaviour. At this moment it allows only
single tunable attribute "unknown":

 <msrs unknown='ignore|fault'/>

Which tells hypervisor to ignore accesses to unimplemented
Model Specific Registers. The only user of that for now is going
to be the bhyve driver.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
6 years agocputest: Use python3 in CPU parser scripts
Jiri Denemark [Thu, 21 Feb 2019 19:28:49 +0000 (20:28 +0100)]
cputest: Use python3 in CPU parser scripts

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agocputest: Adapt scripts to split cpu_map
Jiri Denemark [Thu, 21 Feb 2019 19:10:48 +0000 (20:10 +0100)]
cputest: Adapt scripts to split cpu_map

The tests/cputestdata/cpu-parse.sh script has been broken since the
cpu_map.xml file was split into several XMLs.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agosrc/xenconfig: update copyright notice
David Kiarie [Fri, 22 Feb 2019 11:42:38 +0000 (14:42 +0300)]
src/xenconfig: update copyright notice

Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
6 years agoqemu: fix memory leak in qemuBuildDiskDeviceStr
Ján Tomko [Thu, 21 Feb 2019 15:22:22 +0000 (16:22 +0100)]
qemu: fix memory leak in qemuBuildDiskDeviceStr

Commit a1dce962 added the allocated scsiVPDDeviceId without freeing it.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu: domain: Use VIR_AUTOCLEAN for virBuffer
Peter Krempa [Thu, 21 Feb 2019 15:44:01 +0000 (16:44 +0100)]
qemu: domain: Use VIR_AUTOCLEAN for virBuffer

Replace all uses where virBuffer would need clearing on the cleanup
path.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
6 years agoutil: buffer: Introduce VIR_AUTOCLEAN function for virBuffer
Peter Krempa [Thu, 21 Feb 2019 15:37:50 +0000 (16:37 +0100)]
util: buffer: Introduce VIR_AUTOCLEAN function for virBuffer

virBuffer is almost always stack-allocated, but requires freeing of the
internals on error. Introduce a VIR_AUTOCLEAN function to deal with
this.

Along with the addition add a test which would leak the buffer contents
if it weren't autocleaned.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
6 years agoutil: alloc: Introduce 'VIR_AUTOCLEAN' macros for clearing stack'd structs
Peter Krempa [Thu, 21 Feb 2019 14:49:29 +0000 (15:49 +0100)]
util: alloc: Introduce 'VIR_AUTOCLEAN' macros for clearing stack'd structs

The new utility macros are useful for variables we put on the stack but
require some cleanup. The most prominent of those is virBuffer which is
used almost exclusively in that way.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
6 years agoutil: buf: Remove virBufferEscapeN
Peter Krempa [Thu, 21 Feb 2019 15:29:40 +0000 (16:29 +0100)]
util: buf: Remove virBufferEscapeN

The function was used only in the tests, remove it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
6 years agotests: buf: Fix debug messages in 'testBufEscapeRegex'
Peter Krempa [Thu, 21 Feb 2019 15:26:44 +0000 (16:26 +0100)]
tests: buf: Fix debug messages in 'testBufEscapeRegex'

The messages reference testBufEscapeN instead of testBufEscapeRegex.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
6 years agoutil: buf: Fix memory leak in virBufferEscapeN
Peter Krempa [Thu, 21 Feb 2019 15:07:34 +0000 (16:07 +0100)]
util: buf: Fix memory leak in virBufferEscapeN

The conversion to VIR_AUTOFREE of 'escapeList' introduced memory leak of
the copied item to be escaped:

==17517== 2 bytes in 1 blocks are definitely lost in loss record 1 of 32
==17517==    at 0x483880B: malloc (vg_replace_malloc.c:309)
==17517==    by 0x54D666D: strdup (in /usr/lib64/libc-2.28.so)
==17517==    by 0x497663E: virStrdup (virstring.c:956)
==17517==    by 0x497663E: virStrdup (virstring.c:945)
==17517==    by 0x48F8853: virBufferEscapeN (virbuffer.c:707)
==17517==    by 0x403C9D: testBufEscapeN (virbuftest.c:383)
==17517==    by 0x405FA8: virTestRun (testutils.c:174)
==17517==    by 0x403A70: mymain (virbuftest.c:517)
==17517==    by 0x406BC9: virTestMain (testutils.c:1097)
==17517==    by 0x5470412: (below main) (in /usr/lib64/libc-2.28.so)

[...] (all other have same backtrace as it happens in a loop)

Fix it by reverting all the VIR_AUTO nonsense in this function as there
is exactly one place where it's handled.

This effectively reverts commits:
d0a92a037123085398d4123472f59c71b436d485
96fbf6df90335c1f9315fc25162711fd632d4dab
d261ed2fb1df95f6c7698b9321a82078a7335112

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
6 years agoutil: buffer: Remove misleading AUTOPTR func for 'virBuffer'
Peter Krempa [Thu, 21 Feb 2019 14:39:57 +0000 (15:39 +0100)]
util: buffer: Remove misleading AUTOPTR func for 'virBuffer'

'virBufferFreeAndReset' does not free the top level structure itself.
Additionally we almost exclusively use stack'd buffers rather than
pointers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
6 years agonetwork: add netmask to dhcp range of dnsmasq conf file for IPv4
Laine Stump [Mon, 18 Feb 2019 21:34:50 +0000 (16:34 -0500)]
network: add netmask to dhcp range of dnsmasq conf file for IPv4

dnsmasq documentation says that the *IPv4* prefix/network
address/broadcast address sent to dhcp clients will be automatically
determined by dnsmasq by looking at the interface it's listening on,
so the original libvirt code did not add a netmask to the dnsmasq
commandline (or later, the dnsmasq conf file).

For *IPv6* however, dnsmasq apparently cannot automatically determine
the prefix (functionally the same as a netmask), and it must be
explicitly provided in the conf file (as a part of the dhcp-range
option). So many years after IPv4 DHCP support had been added, when
IPv6 dhcp support was added the prefix was included at the end of the
dhcp-range setting, but only for IPv6.

A user had reported a bug on a host where one of the interfaces was a
superset of the libvirt network where dhcp is needed (e.g., the host's
ethernet is 10.0.0.20/8, and the libvirt network is 10.10.0.1/24). For
some reason dnsmasq was supplying the netmask for the /8 network to
clients requesting an address on the /24 interface.

This seems like a bug in dnsmasq, but even if/when it gets fixed
there, it looks like there is no harm in just always adding the
netmask to all IPv4 dhcp-range options similar to how prefix is added
to all IPv6 dhcp-range options.

Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoutil: set missing data length in virSocketAddrPrefixToNetmask()
Laine Stump [Mon, 18 Feb 2019 21:33:57 +0000 (16:33 -0500)]
util: set missing data length in virSocketAddrPrefixToNetmask()

This fixes a bug that has been present since the original version of
the function was pushed in commit 1ab80f3 on Nov. 26 2010 (by me). The
virSocketAddr::len was not being set.

Apparently until now we were always calling
virSocketAddrPrefixToNetmask with virSocketAddr object that was
already (coincidentally) initialized for the proper address family,
but the bug became apparent when trying to use it to fill in an
otherwise uninitialized object.

Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agosnapshot: Saner use of uuid
Eric Blake [Thu, 21 Feb 2019 15:43:49 +0000 (09:43 -0600)]
snapshot: Saner use of uuid

Most of the code base is fairly consistent about using the name
'uuidstr' when dealing with a formatted human-readable form, and
'uuid' when dealing with the smaller raw bytes form. Fix
snapshot_conf to comply, as well as reducing the scope of a human
string to only the error message that needs it.

Signed-off-by: Eric Blake <eblake@redhat.com>
6 years agoudev: wake up the udev thread for stopping it
Marc Hartmayer [Wed, 20 Feb 2019 10:05:46 +0000 (11:05 +0100)]
udev: wake up the udev thread for stopping it

Signal the udev thread the change of `priv->threadQuit` by using the
thread condition.

Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoudev: nodeStateInitializeEnumerate: remove watch handle in case of an error
Marc Hartmayer [Wed, 20 Feb 2019 10:05:45 +0000 (11:05 +0100)]
udev: nodeStateInitializeEnumerate: remove watch handle in case of an error

If the udev thread is stopped, it must be ensured that the watch
handle is also removed from the main loop.

Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoconf: Use VIR_STEAL_PTR in domain_conf
John Ferlan [Fri, 15 Feb 2019 12:42:17 +0000 (07:42 -0500)]
conf: Use VIR_STEAL_PTR in domain_conf

In preparation for some autofree mods.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agotestutils: Explicitly name virTestCompare*() arguments
Michal Privoznik [Tue, 19 Feb 2019 17:15:23 +0000 (18:15 +0100)]
testutils: Explicitly name virTestCompare*() arguments

Currently, some arguments are called strcontent and strsrc, or
content and src or some other combination. This makes it
impossible to see at the first glance what argument is supposed
to represent 'expected' value and which one represents 'actual'
value. Rename the arguments to make it obvious.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agovirTestCompareToULL: Rename local variables
Michal Privoznik [Wed, 20 Feb 2019 13:14:31 +0000 (14:14 +0100)]
virTestCompareToULL: Rename local variables

The current naming makes it hard for me to see which holds the
expected value and which holds the actual value. Rename them to
make it obvious.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agovirTestCompareToULL: Use VIR_AUTOFREE()
Michal Privoznik [Wed, 20 Feb 2019 13:12:09 +0000 (14:12 +0100)]
virTestCompareToULL: Use VIR_AUTOFREE()

In order to save a few lines of code, and also since it's hype
let's use VIR_AUTOFREE() for the two strings we allocate there.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoiohelper: Remove remaining newlines from error messages
Andrea Bolognani [Tue, 19 Feb 2019 15:03:54 +0000 (16:03 +0100)]
iohelper: Remove remaining newlines from error messages

The iohelper is an internal program that's only supposed to
be called by libvirt, and whatever output it might produce
will ultimately be passed to virReportError() or similar.

Since we do not want strings passed to those functions to
contain newlines, we can simply not output them in the first
place.

This is what happens in pretty much all cases already, but
in a couple instances newlines have managed to slip in.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
6 years agosnapshot: Define explicit flags for snapshot xml
Eric Blake [Wed, 13 Feb 2019 21:09:05 +0000 (15:09 -0600)]
snapshot: Define explicit flags for snapshot xml

Commit f609cb85 (0.9.5) introduced virDomainSnapshotGetXMLDesc()'s use
of @flags as a subset of virDomainXMLFlags, documenting that 2 of the
3 flags defined at the time would never be valid.  Later, commit
28f8dfdc (1.0.0) introduced a new flag, VIR_DOMAIN_XML_MIGRATABLE, but
did not adjust the snapshot documentation to declare it as invalid.
However, since the flag is not accepted as valid by any of the
drivers (remote is just passthrough; esx and vbox don't support flags;
qemu, test, and vz only support VIR_DOMAIN_XML_SECURE), and it is
unlikely that the domain state saved off during a snapshot creation
needs to be migration-friendly (as the snapshot is not the source of
a migration), it is easier to just define an explicit set of supported
flags directly related to the snapshot API rather than trying to
borrow from domain API, and risking confusion if even more domain
flags are added later (in fact, I have an upcoming patch that plans to
add a new flag to virDomainGetXMLDesc that makes no sense for
snapshots).

There is no API or ABI impact (since we purposefully used unsigned int
rather than an enum type in public API, and since the new flag name
carries the same value as the reused name).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agodomain: Define explicit flags for saved image xml
Eric Blake [Wed, 13 Feb 2019 21:09:05 +0000 (15:09 -0600)]
domain: Define explicit flags for saved image xml

Commit d2a929d4 (0.9.4) defined virDomainSaveImageGetXMLDesc()'s use
of @flags as a subset of virDomainXMLFlags, documenting that 2 of the
3 flags defined at the time would never be valid.  Later, commit
28f8dfdc (1.0.0) introduced a new flag, VIR_DOMAIN_XML_MIGRATABLE, but
did not adjust the save image documentation to declare it as invalid.
Later, commit a67e3872 (3.7.0) blindly copied and pasted the same text
into virDomainManagedSaveGetXMLDesc.

However, since the flag is not accepted as valid by any of the
drivers (remote is just passthrough; and qemu is the only supporting
driver for either API, with support for just VIR_DOMAIN_XML_SECURE),
it is easier to just define an explicit set of supported flags
directly related to the save image API rather than trying to borrow
from live domain API, and risking confusion if even more domain flags
are added later (in fact, I have an upcoming patch that plans to add
a new flag to virDomainGetXMLDesc that makes no sense for saved
images).  We may someday decide that saved images need to support the
_MIGRATABLE flag, as it is possible to load a saved image with a
different version of libvirt than the one that created it, but that
can be a separate patch if it is ever needed.  Meanwhile, it DOES make
sense to reuse the same flags for SaveImage and for ManagedSave (since
ManagedSave is really just sugar for creating a normal SaveImage in a
location controlled by libvirt instead of by the user).

There is no API or ABI impact (since we purposefully used unsigned int
rather than an enum type in public API, and since the new flag name
carries the same value as the old reused name).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoqemu: Use correct domain xml flag
Eric Blake [Thu, 14 Feb 2019 18:53:36 +0000 (12:53 -0600)]
qemu: Use correct domain xml flag

Although VIR_DOMAIN_DEF_FORMAT_INACTIVE and VIR_DOMAIN_XML_INACTIVE
happen to have the same value (1<<1), they come from different enums;
and it is nicer to reason about a 'flags' variable if all uses of
that variable are compared against the same enum type.  Messed up in
commit 06f75ff2 (3.8.0).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agodomain: Fix unknown flags diagnosis in virDomainGetXMLDesc
Eric Blake [Thu, 14 Feb 2019 20:25:01 +0000 (14:25 -0600)]
domain: Fix unknown flags diagnosis in virDomainGetXMLDesc

Many drivers had a comment that they did not validate the incoming
'flags' to virDomainGetXMLDesc() because they were relying on
virDomainDefFormat() to do it instead. This used to be the case
(at least since 461e0f1a and friends in 0.9.4 added unknown flag
checking in general), but regressed in commit 0ecd6851 (1.2.12),
when all of the drivers were changed to pass 'flags' through the
new helper virDomainDefFormatConvertXMLFlags(). Since this helper
silently ignores unknown flags, we need to implement flag checking
in each driver instead.

Annoyingly, this means that any new flag values added will silently
be ignored when targeting an older libvirt, rather than our usual
practice of loudly diagnosing an unsupported flag.  Add comments
in domain_conf.[ch] to remind us to be extra vigilant about the
impact when adding flags (a new flag to add data is safe if the
older server omitting the requested data doesn't break things in
the newer client; a new flag to suppress data rather than enhancing
the existing VIR_DOMAIN_XML_SECURE may form a data leak or even a
security hole).

In the qemu driver, there are multiple callers all funnelling to
qemuDomainDefFormatBufInternal(); many of them already validated
flags (and often only a subset of the full set of possible flags),
but for ease of maintenance, we can also check flags at the common
helper function.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoqemu_process: Enter QMP command mode when starting QEMU Process
Chris Venteicher [Thu, 14 Feb 2019 10:25:50 +0000 (11:25 +0100)]
qemu_process: Enter QMP command mode when starting QEMU Process

qemuProcessQMPStart starts a QEMU process and monitor connection that
can be used by multiple functions possibly for multiple QMP commands.

The QMP exchange to exit capabilities negotiation mode and enter command
mode can only be performed once after the monitor connection is
established.

Move responsibility for entering QMP command mode into the
qemuProcessQMP code so multiple functions can issue QMP commands in
arbitrary orders.

This also simplifies the functions using the connection provided by
qemuProcessQMPStart to issue QMP commands.

Test code now needs to call qemuMonitorSetCapabilities to send the
message to switch to command mode because the test code does not use the
qemuProcessQMP command that internally calls qemuMonitorSetCapabilities.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Use unique directories for QMP processes
Chris Venteicher [Wed, 13 Feb 2019 16:22:31 +0000 (17:22 +0100)]
qemu_process: Use unique directories for QMP processes

Multiple QEMU processes for QMP commands can operate concurrently.

Use a unique directory under libDir for each QEMU process to avoid
pidfile and unix socket collision between processes.

The pid file name is changed from "capabilities.pidfile" to "qmp.pid"
because we no longer need to avoid a possible clash with a qemu domain
called "capabilities" now that the processes artifacts are stored in
their own unique temporary directories.

"Capabilities" was changed to "qmp" in the pid file name because these
processes are no longer specific to the capabilities usecase and are
more generic in terms of being used for any general purpose QMP message
exchanges with a QEMU process that is not associated with a domain.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Hide qemuProcessQMPStop
Jiri Denemark [Wed, 13 Feb 2019 16:18:51 +0000 (17:18 +0100)]
qemu_process: Hide qemuProcessQMPStop

Users qemuProcessQMP struct were always forced to call both
qemuProcessQMPStop and qemuProcessQMPFree when they are done with the
process. We can just call qemuProcessQMPStop from qemuProcessQMPFree and
let users call qemuProcessQMPFree only.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Document and cleanup qemuProcessQMPNew
Chris Venteicher [Sun, 13 Jan 2019 00:50:15 +0000 (18:50 -0600)]
qemu_process: Document and cleanup qemuProcessQMPNew

qemuProcessQMPNew is one of the public functions used to create and
manage a QEMU process for QMP command exchanges outside of domain
operations.

Add descriptive comment block, debug statement and make source
consistent with the cleanup / VIR_STEAL_PTR format used elsewhere.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Stop retaining monitor config in qemuProcessQMP
Chris Venteicher [Sun, 13 Jan 2019 00:50:14 +0000 (18:50 -0600)]
qemu_process: Stop retaining monitor config in qemuProcessQMP

The monitor config data is removed from the qemuProcessQMP struct.

The monitor config data can be initialized immediately before call to
qemuMonitorOpen and does not need to be maintained after the call
because qemuMonitorOpen copies any strings it needs.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Setup paths within qemuProcessQMPInit
Chris Venteicher [Sun, 13 Jan 2019 00:50:13 +0000 (18:50 -0600)]
qemu_process: Setup paths within qemuProcessQMPInit

Move code for setting paths and prepping file system from
qemuProcessQMPNew to qemuProcessQMPInit.

This keeps qemuProcessQMPNew limited to data structures and path
initialization is done in qemuProcessQMPInit.

The patch is a non-functional, cut / paste change, however goto is now
"cleanup" rather than "error".

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Store libDir in qemuProcessQMP struct
Chris Venteicher [Sun, 13 Jan 2019 00:50:12 +0000 (18:50 -0600)]
qemu_process: Store libDir in qemuProcessQMP struct

Store libDir path in the qemuProcessQMP struct in anticipation of moving
path construction code into qemuProcessQMPInit function.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Move monitor code to qemuProcessQMPConnectMonitor
Chris Venteicher [Sun, 13 Jan 2019 00:50:11 +0000 (18:50 -0600)]
qemu_process: Move monitor code to qemuProcessQMPConnectMonitor

All code related to QEMU monitor is moved from qemuProcessQMPNew and
qemuProcessQMPInit into qemuProcessQMPConnectMonitor.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Introduce qemuProcessQMPStart
Chris Venteicher [Sun, 13 Jan 2019 00:50:10 +0000 (18:50 -0600)]
qemu_process: Introduce qemuProcessQMPStart

This is a replacement for qemuProcessQMPRun to make the name consistent
with qemuProcessStart. The original qemuProcessQMPRun function is
renamed as qemuProcessQMPLaunch and becomes one of the simpler functions
called from the main qemuProcessQMPStart entry point. The following
patches will move parts of the code in qemuProcessQMPLaunch to the other
functions (qemuProcessQMPInit and qemuProcessQMPConnectMonitor).

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Hide qmperr inside qemuProcessQMP
Jiri Denemark [Tue, 12 Feb 2019 14:00:42 +0000 (15:00 +0100)]
qemu_process: Hide qmperr inside qemuProcessQMP

Keep the pointer to QEMU stderr output in qemuProcessQMP struct instead
of requiring the caller to provide it (and free it).

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_capabilities: Log probe failure in virQEMUCapsInitQMPSingle
Jiri Denemark [Tue, 12 Feb 2019 13:50:39 +0000 (14:50 +0100)]
qemu_capabilities: Log probe failure in virQEMUCapsInitQMPSingle

Let's push the call to virQEMUCapsLogProbeFailure down the stack to
where the probing failure is detected.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Don't ignore errors in virQEMUCapsInit
Jiri Denemark [Tue, 12 Feb 2019 13:38:40 +0000 (14:38 +0100)]
qemu_process: Don't ignore errors in virQEMUCapsInit

While qemuProcessQMPRun and virQEMUCapsInitQMPMonitor* functions called
from virQEMUCapsInit ignore some errors, the caller of virQEMUCapsInit
would report an error unless usedQMP is true anyway. And since usedQMP
can only be true if the probing code really succeeded (i.e., no errors
were ignored), we can just simplify the logic by not ignoring the errors
in the first place.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_capabilities: Refactor virQEMUCapsInitQMP
Jiri Denemark [Mon, 11 Feb 2019 16:06:31 +0000 (17:06 +0100)]
qemu_capabilities: Refactor virQEMUCapsInitQMP

The function contains two almost identical parts. Let's consolidate them
into a single helper function and call it twice.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Use qemuProcessQMP struct for a single process
Chris Venteicher [Sun, 13 Jan 2019 00:50:06 +0000 (18:50 -0600)]
qemu_process: Use qemuProcessQMP struct for a single process

In new process code, move from model where qemuProcessQMP struct can be
used to activate a series of Qemu processes to model where one
qemuProcessQMP struct is used for one and only one Qemu process.

By allowing only one process activation per qemuProcessQMP struct, the
struct can safely store process outputs like status and stderr, without
being overwritten, until qemuProcessQMPFree is called.

By doing this, process outputs like status and stderr can remain stored
in the qemuProcessQMP struct without being overwritten by subsequent
process activations.

The forceTCG parameter (use / don't use KVM) will be passed when the
qemuProcessQMP struct is initialized since the qemuProcessQMP struct
won't be reused.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_capabilities: Stop QEMU process before freeing
Chris Venteicher [Sun, 13 Jan 2019 00:50:05 +0000 (18:50 -0600)]
qemu_capabilities: Stop QEMU process before freeing

virQEMUCapsInitQMP now stops QEMU process in all execution paths,
before freeing the process structure.

The qemuProcessQMPStop function can be called multiple times without
problems... Won't attempt to stop processes and free resources multiple
times.

Follow the convention established in qemu_process of
1) alloc process structure
2) start process
3) use process
4) stop process
5) free process data structure

The process data structure persists after the process activation fails
or the process dies or is killed so stderr strings can be retrieved
until the process data structure is freed.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Use consistent name for stop process function
Chris Venteicher [Sun, 13 Jan 2019 00:50:04 +0000 (18:50 -0600)]
qemu_process: Use consistent name for stop process function

s/qemuProcessQMPAbort/qemuProcessQMPStop/ applied to change function
name used to stop QEMU processes in process code moved from
qemu_capabilities.

No functionality change.

The new name, qemuProcessQMPStop, is consistent with the existing
function qemuProcessStop used to stop Domain processes.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Refer to proc not cmd in process code
Chris Venteicher [Sun, 13 Jan 2019 00:50:03 +0000 (18:50 -0600)]
qemu_process: Refer to proc not cmd in process code

s/cmd/proc/ in process code imported from qemu_capabilities.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Limit qemuProcessQMPNew to const input strings
Chris Venteicher [Sun, 13 Jan 2019 00:50:02 +0000 (18:50 -0600)]
qemu_process: Limit qemuProcessQMPNew to const input strings

Add the const qualifier on non modified strings
(string only copied inside qemuProcessQMPNew)
so that const strings can be used directly in calls to
qemuProcessQMPNew in future patches.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Rename identifiers moved from qemu_capabilities
Chris Venteicher [Sun, 13 Jan 2019 00:50:01 +0000 (18:50 -0600)]
qemu_process: Rename identifiers moved from qemu_capabilities

s/virQEMUCapsInitQMPCommand/qemuProcessQMP/

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoqemu_process: Move process code from qemu_capabilities
Chris Venteicher [Sun, 13 Jan 2019 00:50:00 +0000 (18:50 -0600)]
qemu_process: Move process code from qemu_capabilities

QEMU process code in qemu_capabilities.c is moved to qemu_process.c in
order to make the code usable outside the original capabilities use
cases.

The moved code activates and manages QEMU processes without establishing
a guest domain.

This patch is a straight cut/paste move between files.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agovirsh: fix return value in storage vol name completor
Daniel P. Berrangé [Mon, 11 Feb 2019 14:21:35 +0000 (14:21 +0000)]
virsh: fix return value in storage vol name completor

The function must return a pointer, not a boolean. Fortunately 'false'
is equivalent to 'NULL' so this bug no had ill effect previously.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
6 years agoconf: make virPCIDeviceAddressFormat void
Daniel P. Berrangé [Wed, 19 Dec 2018 11:58:42 +0000 (11:58 +0000)]
conf: make virPCIDeviceAddressFormat void

Only one of the three callers of virPCIDeviceAddressFormat correctly
handles an error return status. Fortunately it can't fail so can be
made void.

Reviewed-by: Laine Stump <laine@laine.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
6 years agoiohelper: Don't include newlines in error messages
Andrea Bolognani [Tue, 5 Feb 2019 14:54:55 +0000 (15:54 +0100)]
iohelper: Don't include newlines in error messages

The newline was pretty arbitrary, and we're better off
without it.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agovircommand: Ensure buffers are NULL-terminated
Andrea Bolognani [Tue, 5 Feb 2019 13:30:42 +0000 (14:30 +0100)]
vircommand: Ensure buffers are NULL-terminated

The memory allocated by VIR_REALLOC_N() is uninitialized,
which means it's not possible to figure out whether any
output was produced at all after the fact.

Since we don't care about the previous contents of buffers,
if any, use VIR_FREE() followed by VIR_ALLOC_N() instead.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
6 years agovz: build fix for virStorageBackendVzPoolStart
Nikolay Shirokovskiy [Thu, 14 Feb 2019 13:06:54 +0000 (16:06 +0300)]
vz: build fix for virStorageBackendVzPoolStart

Remove unused variable. Fix for [1]

[1] 821dd6d8: storage: Use VIR_AUTOFREE for storage backends

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
6 years agoRemove remaining references to kqemu
Ján Tomko [Mon, 18 Feb 2019 15:13:30 +0000 (16:13 +0100)]
Remove remaining references to kqemu

We dropped support in commit 8e91a40 (November 2015), but some
occurrences still remained, even in live code.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
6 years agoutil: Use virStorageSourceNew in virStorageFileMetadataNew
Peter Krempa [Mon, 18 Feb 2019 12:24:37 +0000 (13:24 +0100)]
util: Use virStorageSourceNew in virStorageFileMetadataNew

Commit dcda2bf4c110 forgot to fix this one instance.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoutil: Replace virStorageSourceFree with virObjectUnref
Peter Krempa [Fri, 15 Feb 2019 12:03:58 +0000 (13:03 +0100)]
util: Replace virStorageSourceFree with virObjectUnref

Now that virStorageSource is a subclass of virObject we can use
virObjectUnref and remove virStorageSourceFree which was a thin wrapper.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
6 years agoutil: Remove the AUTOPTR func for virStorageSource
Peter Krempa [Fri, 15 Feb 2019 11:52:45 +0000 (12:52 +0100)]
util: Remove the AUTOPTR func for virStorageSource

Since virStorageSource is now a subclass of virObject, we can use
VIR_AUTOUNREF instead.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
6 years agoutil: alloc: Introduce VIR_AUTOUNREF macro
Peter Krempa [Fri, 15 Feb 2019 11:35:12 +0000 (12:35 +0100)]
util: alloc: Introduce VIR_AUTOUNREF macro

Add helper for utilizing __attribute__(cleanup())) for unref-ing
instances of sublasses of virObject.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
6 years agoutil: storage: Turn virStorageSource into a virObject
Peter Krempa [Fri, 15 Feb 2019 08:46:03 +0000 (09:46 +0100)]
util: storage: Turn virStorageSource into a virObject

To allow tracking a single virStorageSource in multiple structures
without extra hassle allow refcounting by turining it into an object.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
6 years agoutil: Introduce function for allocating virStorageSource
Peter Krempa [Thu, 14 Feb 2019 15:20:25 +0000 (16:20 +0100)]
util: Introduce function for allocating virStorageSource

Add virStorageSourceNew and refactor places allocating that structure to
use the helper.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
6 years agobhyve: use virDomainDiskDefNew to instead of VIR_ALLOC
Peter Krempa [Mon, 18 Feb 2019 09:12:24 +0000 (10:12 +0100)]
bhyve: use virDomainDiskDefNew to instead of VIR_ALLOC

Use the proper function to allocate a disk definition.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
6 years agoconf: Rework virDomainDeviceDefPostParseCommon()
Andrea Bolognani [Fri, 15 Feb 2019 11:39:36 +0000 (12:39 +0100)]
conf: Rework virDomainDeviceDefPostParseCommon()

Now that we've moved all the actual code into helper
functions, we can turn it into a switch statement.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoconf: Introduce virDomainNetDefPostParse()
Andrea Bolognani [Fri, 15 Feb 2019 11:26:13 +0000 (12:26 +0100)]
conf: Introduce virDomainNetDefPostParse()

Minor tweaks to ensure compliance with our coding style.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoconf: Introduce virDomainControllerDefPostParse()
Andrea Bolognani [Fri, 15 Feb 2019 11:22:24 +0000 (12:22 +0100)]
conf: Introduce virDomainControllerDefPostParse()

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoconf: Introduce virDomainVideoDefPostParse()
Andrea Bolognani [Fri, 15 Feb 2019 11:17:06 +0000 (12:17 +0100)]
conf: Introduce virDomainVideoDefPostParse()

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoconf: Introduce virDomainDiskDefPostParse()
Andrea Bolognani [Fri, 15 Feb 2019 11:05:51 +0000 (12:05 +0100)]
conf: Introduce virDomainDiskDefPostParse()

Minor tweaks to ensure compliance with our coding style.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoconf: Introduce virDomainRNGDefPostParse()
Andrea Bolognani [Fri, 15 Feb 2019 09:39:20 +0000 (10:39 +0100)]
conf: Introduce virDomainRNGDefPostParse()

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoconf: Introduce virDomainChrDefPostParse()
Andrea Bolognani [Fri, 15 Feb 2019 09:35:02 +0000 (10:35 +0100)]
conf: Introduce virDomainChrDefPostParse()

Minor tweaks to ensure compliance with our coding style.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agonetwork: explicitly allow icmp/icmpv6 in libvirt zonefile
Laine Stump [Thu, 14 Feb 2019 19:33:34 +0000 (14:33 -0500)]
network: explicitly allow icmp/icmpv6 in libvirt zonefile

The libvirt zonefile for firewalld (added in commit 3b71f2e4) does the
following:

1) lists specific services it wants to allow, then

2) uses a lower priority <reject/> rule to block all other services to
   the host, and then finally,

3) relies on the zone's default "accept" policy to, accept all
   forwarded traffic (since forwarded traffic is ignored by the
   slightly higher priority <reject/> rule in (2)).

I had assumed that icmp traffic was either being allowed at the top of
the rules, or that it would be ignored by the <reject/> rule and
passed by the default accept policy (similar to forwarded traffic),
but this assumption was incorrect; the <reject/> rule does block icmp
traffic. This became apparent when DHCPv6 which requires ICMPv6 in
addition to udp/dhcpv6) failed to work.

This all means that in order to achieve our original goal of "similar
behavior to a default reject policy, but also allowing forwarded
traffic", we need to add rules to allow all icmp and icmpv6 traffic to
the libvirt zone, and that's what this patch does.

This is a further refinement of the resolution to
https://bugzilla.redhat.com/1650320

Signed-off-by: Laine Stump <laine@laine.org>
Acked-by: Eric Garver <eric@garver.life>
6 years agovirkmodtest: Don't fail if modprobe doesn't exist
Michal Privoznik [Thu, 14 Feb 2019 15:25:44 +0000 (16:25 +0100)]
virkmodtest: Don't fail if modprobe doesn't exist

On some very basic installations (e.g. some container images) the
modprobe binary might be missing. If that is the case, don't fail
virkmodtest.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
6 years agovirsh: fix snapshot list --parent
Ján Tomko [Thu, 14 Feb 2019 13:45:12 +0000 (14:45 +0100)]
virsh: fix snapshot list --parent

The root snapshot does not have a parent.
Use NULLSTR_EMPTY to pass an empty string instead of putting
too few columns in the table.

https://bugzilla.redhat.com/show_bug.cgi?id=1662849

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
6 years agoqemu_hotplug: Initialize @charAlias in qemuDomainRemoveChrDevice
Michal Privoznik [Thu, 14 Feb 2019 13:13:08 +0000 (14:13 +0100)]
qemu_hotplug: Initialize @charAlias in qemuDomainRemoveChrDevice

My change in 112f3a8d0f32 was too drastic. The @charAlias
variable is initialized only if @monitor == true. However, it is
used even outside of that condition, at which point it's just
uninitialized pointer.

Reported-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoudev: only report a warning if udev_enumerate_scan_devices fails
Marc Hartmayer [Thu, 14 Feb 2019 09:01:01 +0000 (10:01 +0100)]
udev: only report a warning if udev_enumerate_scan_devices fails

Even if an error is reported by `udev_enumerate_scan_devices`,
e.g. because a driver of a device has an bug, we can still enumerate
all other devices. Additionally the documentation of
udev_enumerate_scan_devices says that on success an integer >= 0 is
returned (see man udev_enumerate_scan_devices(3)).

Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoUse NULLSTR_EMPTY
Ján Tomko [Tue, 12 Feb 2019 16:25:06 +0000 (17:25 +0100)]
Use NULLSTR_EMPTY

Instead of repetitive:
  s ? s : ""
use NULLSTR_EMPTY.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
6 years agoRemove EMPTY_STR macro
Ján Tomko [Tue, 12 Feb 2019 16:15:17 +0000 (17:15 +0100)]
Remove EMPTY_STR macro

Another misleadingly named macro.
Deprecate in favor of NULLSTR_STAR.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
6 years agoRemove EMPTYSTR macro
Ján Tomko [Tue, 12 Feb 2019 16:11:11 +0000 (17:11 +0100)]
Remove EMPTYSTR macro

This macro neither takes nor produces an empty string.
Remove it in favor of NULLSTR_MINUS.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
6 years agotools: use NULLSTR_MINUS
Ján Tomko [Tue, 12 Feb 2019 16:09:49 +0000 (17:09 +0100)]
tools: use NULLSTR_MINUS

Use the newly introduced macro in the few places that open-code it.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
6 years agointernal: introduce a family of NULLSTR macros
Ján Tomko [Tue, 12 Feb 2019 16:01:09 +0000 (17:01 +0100)]
internal: introduce a family of NULLSTR macros

NULLSTR_EMPTY, the quiet child,
NULLSTR_STAR, the famous one and
NULLSTR_MINUS, the grumpy one.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
6 years agoqemu_hotplug: Assume chardev alias always exists in qemuDomainDetachChrDevice
Michal Privoznik [Thu, 14 Feb 2019 09:44:15 +0000 (10:44 +0100)]
qemu_hotplug: Assume chardev alias always exists in qemuDomainDetachChrDevice

The @tmpChr is looked up in domain definition based on user
provided chardev XML. Therefore, the alias must have been
allocated already when domain was started up.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoqemu_hotplug: Don't build device string in qemuDomainDetachChrDevice
Michal Privoznik [Thu, 14 Feb 2019 09:38:09 +0000 (10:38 +0100)]
qemu_hotplug: Don't build device string in qemuDomainDetachChrDevice

This is basically an old artefact from 24b0821926e when the idea
was:

1) Build device string only to see if chardev has any -device
associated with it and thus if device_del is needed
2) Detach chardev using chardev_del

Now, that DEVICE and DEVICE_DELETED capabilities are assumed for
every domain 1) does not make sense anymore.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoqemuhotplugtest: Test guestfwd attach and detach
Michal Privoznik [Mon, 11 Feb 2019 15:17:53 +0000 (16:17 +0100)]
qemuhotplugtest: Test guestfwd attach and detach

Previous two commits demonstrate a hole in our test scenario.
Fix that.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoqemu_hotplug: Detach guestfwd using netdev_del
Michal Privoznik [Mon, 11 Feb 2019 13:16:58 +0000 (14:16 +0100)]
qemu_hotplug: Detach guestfwd using netdev_del

https://bugzilla.redhat.com/show_bug.cgi?id=1624204

The guestfwd channels are -netdevs really. Hotunplug them as
such. Also, DEVICE_DELETED event is not triggered (surprisingly,
since we're not issuing device_del rather than netdev_del) and
associated chardev is removed automagically too. This means that
we need to do qemuDomainRemoveChrDevice() minus monitor call to
remove the chardev.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoqemu_hotplug: Attach guestfwd using netdev_add
Michal Privoznik [Mon, 11 Feb 2019 15:05:37 +0000 (16:05 +0100)]
qemu_hotplug: Attach guestfwd using netdev_add

https://bugzilla.redhat.com/show_bug.cgi?id=1624204

The guestfwd channels are -netdevs really. Hotplug them as such.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoqemuL: Drop "user-" prefix for guestfwd netdev
Michal Privoznik [Mon, 11 Feb 2019 13:16:00 +0000 (14:16 +0100)]
qemuL: Drop "user-" prefix for guestfwd netdev

Introduced by d86c876a66e3.

There is no real need to have "user-" prefix for chardev.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoqemu: Use @tmpChr in qemuDomainDetachChrDevice to build device string
Michal Privoznik [Mon, 11 Feb 2019 13:13:39 +0000 (14:13 +0100)]
qemu: Use @tmpChr in qemuDomainDetachChrDevice to build device string

So far we are passing @chr to qemuBuildChrDeviceStr. This is
suboptimal (in fact wrong) because @chr is just parsed XML
definition provided by user which by definition may lack some
information. On the other hand, @tmpChr is the one that was found
using @chr in domain definition so it contains the same amount of
information or more.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoqemu: Escape external snapshot names containing comma
Eric Blake [Wed, 13 Feb 2019 04:18:15 +0000 (22:18 -0600)]
qemu: Escape external snapshot names containing comma

The code for creating external snapshots for an offline domain
called out to qemu-img without escaping commas in the manner
that qemu-img expects. This also fixes a typo in the comment.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoutil: fix memory leak in virFirewallDInterfaceSetZone()
Laine Stump [Wed, 13 Feb 2019 15:57:57 +0000 (10:57 -0500)]
util: fix memory leak in virFirewallDInterfaceSetZone()

commit 3bba4825 added the new function virFirewallDInterfaceSetZone()
which calledsends virDBUSCallMethod a DBusMessage** for the reply
message, but doesn't use the reply, and also doesn't free it. Since
this arg is allowed to be NULL, this patch simply sets it to NULL so
we don't have to deal with it.

Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
6 years agovirsh: initialize info in cmdIOThreadInfo
Ján Tomko [Tue, 12 Feb 2019 11:04:24 +0000 (12:04 +0100)]
virsh: initialize info in cmdIOThreadInfo

Although it is not needed at the moment, do not rely on a value being
set before the first jump to cleanup.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agovirsh: remove redundant virshNodeGetCPUCount
Ján Tomko [Tue, 12 Feb 2019 10:52:59 +0000 (11:52 +0100)]
virsh: remove redundant virshNodeGetCPUCount

Since commit 4c4b821e it is not used for anything.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agovirsh: do not assign negative values to niothreads
Ján Tomko [Tue, 12 Feb 2019 10:42:36 +0000 (11:42 +0100)]
virsh: do not assign negative values to niothreads

Use a temporary 'rc' variable to avoid comparing signed
and unsigned integers in the cleanup section.

Bug introduced by commit 3072ded which added the comparison against
the unsigned 'i'.

Also make niothreads size_t to mark that it should be unsigned.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agovirsh: reduce the optimism in cmdIOThreadInfo
Ján Tomko [Tue, 12 Feb 2019 09:35:17 +0000 (10:35 +0100)]
virsh: reduce the optimism in cmdIOThreadInfo

Instead of using niothreads which defaults to zero, use the common
pattern with a ret varaible set to true just before the cleanup label.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
6 years agoutil: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageSource
John Ferlan [Tue, 12 Feb 2019 18:17:56 +0000 (13:17 -0500)]
util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageSource

Let's make use of the auto __cleanup capabilities cleaning up any
now unnecessary goto paths.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoutil: Check for duplicated id in virStorageSourceParseRBDColonString
John Ferlan [Tue, 12 Feb 2019 13:36:40 +0000 (08:36 -0500)]
util: Check for duplicated id in virStorageSourceParseRBDColonString

If we find multiple "id=" strings during processing, then we need
to force an error since we cannot have multiple <auth>'s defined
for a single source volume.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agoconf: Check for duplicate authdef during hostdev iSCSI processing
John Ferlan [Tue, 12 Feb 2019 13:35:38 +0000 (08:35 -0500)]
conf: Check for duplicate authdef during hostdev iSCSI processing

If virDomainHostdevSubsysSCSIiSCSIDefParseXML processing finds a
duplicated <auth> structure, we should error out rather than continue.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agotests: Fix memory leak in testCompareXMLToArgvFiles
John Ferlan [Fri, 8 Feb 2019 15:43:23 +0000 (10:43 -0500)]
tests: Fix memory leak in testCompareXMLToArgvFiles

Only one path will consume the @def; otherwise, we need to free it.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agostorage: Use VIR_AUTOCLOSE
John Ferlan [Thu, 31 Jan 2019 23:56:27 +0000 (18:56 -0500)]
storage: Use VIR_AUTOCLOSE

Modify code to use the VIR_AUTOCLOSE logic cleaning up any
now unnecessary goto paths.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
6 years agostorage: Rework ret logic in storageBackendUpdateVolTargetInfo
John Ferlan [Tue, 12 Feb 2019 12:46:38 +0000 (07:46 -0500)]
storage: Rework ret logic in storageBackendUpdateVolTargetInfo

Rather than overload @ret with trying serve multiple purposes,
let's initialize @ret to -1 and introduce an @rc function return
value that can be used for functions that may return -1 or -2
and only override @ret when rc < 0.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>