I wanted to do a demonstration with virsh batch mode, which
takes multiple commands all packed into a single argument:

$ virsh -c test:///default 'echo a; echo b;'
a
b

but that produced a really long line, so I tried to make it
more legible:

$ virsh -c test:///default '
   echo a;
   echo b;
'
error: unknown command: '
'

Let's be more like the shell, and treat unquoted newline as a
command separator just as we do for semicolon.  In fact, with
that, I can even now mix styles:

$ virsh -c test:///default '
   echo a; echo b
   echo c
'
a
b
c
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

vcpupin will fail when maxvcpus is larger than current
vcpu:

virsh vcpupin win7 --vcpu 0 --cpulist 5-6
error: Requested operation is not valid: cpu affinity is not supported

win7 xml in the command above is like below:
...
<vcpu current="3" placement="static">8</vcpu>
...

The reason is vcpu[3] and vcpu[4] have zero tids and should not been
compared as valid situation in qemuDomainRefreshVcpuInfo().

This issue is introduced by commit 34f77437, which fix recording of vCPU
pids for MTTCG.
Signed-off-by: NYi Wang <wang.yi59@zte.com.cn>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Added GPFS as shared file system recognized during live migration
security checks.

GPFS is 'IBM General Parallel File System' also called
'IBM Spectrum Scale'

BUG: https://bugzilla.redhat.com/show_bug.cgi?id=1679528Signed-off-by: NDiego Michelotto <diego.michelotto@cnaf.infn.it>
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>

The structure used to handle network entries was based on 'if,else'
conditions. This commit converts this ugly structure into a switch to
clearify each option of the handler.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Extract out the network "type" processing into it's own method
rather than inline within lxcNetworkParseDataSuffix.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

This commit removes the full network entry setting: "lxc.network.X" to
type only. Like "type", "name", "flags", etc. This will handle entries
regardless of whether they are prefixed by "lxc.network." (today) or
"lxc.net.X." (the future).
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Refactor lxcNetworkWalkCallback to be a simple method to handle
both possible network settings with indexes or the simple one. It is
better the decouple the whole algorithm to parse data to only parse
which entry type libvirt is handling.

The new method is responsible to verify is the settings correspond to
network entry. Right now, it is only verifying "lxc.network.", but in
the future, it can be used to verify "lxc.net.X." too. Any other case
would be rejected.

On the other hand, the idea here is working only with types. If we know
that entry is part of network settings, after we just need to know which
type is. It keeps the handler simple.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The new method called lxcNetworkParseDataIPs() is responsible to handle
IPv{4,6} settings now. The idea is let lxcNetworkWalkCallback() method
handle all entries related to network definition only.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

libvirt_iohelper is used internally by the virFileWrapperFd APIs;
more specifically, in the QEMU driver we have the doCoreDump() and
qemuDomainSaveMemory() helper functions as users, and those in turn
end up being called by the implementation of several driver APIs.

By calling virReportError() if libvirt_iohelper has failed, we
overwrite whatever generic error message QEMU might have raised
with the more useful one generated by the helper program.

After this commit, the user will be able to see the error directly
instead of having to dig in the journal or libvirtd log.

https://bugzilla.redhat.com/show_bug.cgi?id=1578741Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

virFileWrapperFdFree(), like all free functions, is supposed
to only release allocated resources, so error reporting is
better suited for virFileWrapperFdClose().

This reverts commit b0c3e931.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Right now we're reporting errors in virFileWrapperFdFree(),
but that's hardly the appropriate place to do so, as free
functions are supposed to do nothing more than release
allocated resources.

We want to move that code back into virFileWrapperFdClose(),
but before we can do that we need to make sure the function
is actually called every time we're done processing the
wrapped file. The cleanup path is the obvious candidate.

In a couple of cases we can just move the call, but for the
remaining ones we need to duplicate it instead in order not
to alter the existing behavior. We do, however, make sure
that in all cases a failure to properly close the wrapper
results in the overall operation being reported as failed.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

We'll want to use this function in the cleanup path soon,
and in order to be able to do that we need to make sure we
can call it multiple times on the same virFileWrapperFd
without side effects.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Replace virDomainChrSourceDefFree with virObjectUnref.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Use refcounting for priv->monConfig instead of asymmetric freeing.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Change fb01e1a4 "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/1815452Acked-by: NJamie Strandboge <jamie@canonical.com>
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

Change fb01e1a4 "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: NJamie Strandboge <jamie@canonical.com>
Signed-off-by: NChristian Ehrhardt <christian.ehrhardt@canonical.com>

Add a bhyveDomainDefNeedsISAController() helper function
which by domain configuration determines whether LPC controller is
required or not.
Signed-off-by: NRoman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Describe bhyve's ignoring unknown MSRs access feature
introduced by commit e9528f41.
Signed-off-by: NRoman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

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: NRoman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

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: NRoman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The tests/cputestdata/cpu-parse.sh script has been broken since the
cpu_map.xml file was split into several XMLs.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NDavid Kiarie <davidkiarie4@gmail.com>

Commit a1dce962 added the allocated scsiVPDDeviceId without freeing it.
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Replace all uses where virBuffer would need clearing on the cleanup
path.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

The function was used only in the tests, remove it.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

The messages reference testBufEscapeN instead of testBufEscapeRegex.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

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:
d0a92a03
96fbf6df
d261ed2fSigned-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

'virBufferFreeAndReset' does not free the top level structure itself.
Additionally we almost exclusively use stack'd buffers rather than
pointers.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

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: NLaine Stump <laine@laine.org>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

This fixes a bug that has been present since the original version of
the function was pushed in commit 1ab80f32 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: NLaine Stump <laine@laine.org>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

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: NEric Blake <eblake@redhat.com>

Signal the udev thread the change of `priv->threadQuit` by using the
thread condition.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

If the udev thread is stopped, it must be ensured that the watch
handle is also removed from the main loop.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

In preparation for some autofree mods.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

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: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

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: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

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: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>