# virsh -c lxc:/// start helloworld
 error: Failed to start domain helloworld
 error: internal error: guest failed to start: Unknown
 failure in libvirt_lxc startup

Return success when there are no cpuset.mems to be set,
instead of failing without setting an error.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
(cherry picked from commit 930e8697)

 # virsh -c lxc:/// start helloworld
 error: Failed to start domain helloworld
 error: internal error: guest failed to start: Invalid value '1-3'
 for 'cpuset.mems': Invalid argument

Free the cpu mask to avoid reusing it as a mem mask
in virCgroupSetCpusetMems
if virDomainNumatuneMaybeFormatNodeset does not set a mask.
Signed-off-by: NLuyao Huang <lhuang@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>
(cherry picked from commit 461eafec)

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

Commit a0670aef caused a regression in 'virsh event' and
'virsh qemu-monitor-event' - if a user tries to filter the
command to a specific domain, an error message is printed:

$ virsh event dom --loop
error: internal error: virsh qemu-monitor-event: no domain VSH_OT_DATA option

and then the command continues as though no domain had been
supplied (giving events for ALL domains, instead of the
requested one).  This is because the code was incorrectly
assuming that all "domain" options would be supplied via a
mandatory VSH_OT_DATA, even though "domain" is optional for
these two commands, so we had changed them to VSH_OT_STRING
to quit failing for other reasons (ever since it was decided
that VSH_OT_DATA and VSH_OT_STRING should no longer be
synonyms).

In looking at the situation, though, the code for looking up
a domain was making a pointless check for whether the option
exists prior to finding the option's string value, as
vshCommandOptStringReq does just fine at reporting any errors
when looking up a string whether or not the option was present.

So this is a case of regression fixing by pure code deletion :)

* tools/virsh-domain.c (vshCommandOptDomainBy): Drop useless filter.
* tools/virsh-interface.c (vshCommandOptInterfaceBy): Likewise.
* tools/virsh-network.c (vshCommandOptNetworkBy): Likewise.
* tools/virsh-nwfilter.c (vshCommandOptNWFilterBy): Likewise.
* tools/virsh-secret.c (vshCommandOptSecret): Likewise.
* tools/virsh.h (vshCmdHasOption): Drop unused function.
* tools/virsh.c (vshCmdHasOption): Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>
(cherry picked from commit 31ef0836)

$ sudo virsh change-media f19 hdc /mnt/data/devel/media/Fedora-16-x86_64-Live-KDE.iso
succeeded to complete action update on media

Change the message to:

  Successfully {inserted,ejected,changed} media.

https://bugzilla.redhat.com/show_bug.cgi?id=967946
(cherry picked from commit e3aa4c91)

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

Even though we have a mutex mechanism so that two clients don't spawn
two daemons, it's not strong enough. It can happen that while one
client is spawning the daemon, the other one fails to connect.
Basically two possible errors can happen:

  error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': Connection refused

or:

  error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': No such file or directory

The problem in both cases is, the daemon is only starting up, while we
are trying to connect (and fail). We should postpone the connecting
phase until the daemon is started (by the other thread that is
spawning it). In order to do that, create a file lock 'libvirt-lock'
in the directory where session daemon would create its socket. So even
when called from multiple processes, spawning a daemon will serialize
on the file lock. So only the first to come will spawn the daemon.
Tested-by: NRichard W. M. Jones <rjones@redhat.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
(cherry picked from commit be78814a)

While this thread is cleaning up the client and connection objects:
 #2  virFileReadAll (path=0x7f28780012b0 "/proc/1319/stat", maxlen=maxlen@entry=1024, buf=buf@entry=0x7f289c60fc40) at util/virfile.c:1287
 #3  0x00007f28adbb1539 in virProcessGetStartTime (pid=<optimized out>, timestamp=timestamp@entry=0x7f289c60fc98) at util/virprocess.c:838
 #4  0x00007f28adb91981 in virIdentityGetSystem () at util/viridentity.c:151
 #5  0x00007f28ae73f17c in remoteClientFreeFunc (data=<optimized out>) at remote.c:1131
 #6  0x00007f28adcb7f33 in virNetServerClientDispose (obj=0x7f28aecad180) at rpc/virnetserverclient.c:858
 #7  0x00007f28adba8eeb in virObjectUnref (anyobj=<optimized out>) at util/virobject.c:265
 #8  0x00007f28ae74ad05 in virNetServerHandleJob (jobOpaque=<optimized out>, opaque=0x7f28aec93ff0) at rpc/virnetserver.c:205
 #9  0x00007f28adbbef4e in virThreadPoolWorker (opaque=opaque@entry=0x7f28aec88030) at util/virthreadpool.c:145

In stack frame #6 the client->identity object got unref'd, but the code
that removes the event callbacks in frame #5 did not run yet as we are
trying to obtain the system identity (frames #4, #3, #2).

In other thead:
 #0  virObjectUnref (anyobj=anyobj@entry=0x7f288c162c60) at util/virobject.c:264
        klass = 0xdeadbeef
        obj = 0x7f288c162c60
 #1  0x00007f28ae71c709 in remoteRelayDomainEventCheckACL (client=<optimized out>, conn=<optimized out>, dom=dom@entry=0x7f28aecaafc0) at remote.c:164
 #2  0x00007f28ae71fc83 in remoteRelayDomainEventTrayChange (conn=<optimized out>, dom=0x7f28aecaafc0, ... ) at remote.c:717
 #3  0x00007f28adc04e53 in virDomainEventDispatchDefaultFunc (conn=0x7f287c0009a0, event=0x7f28aecab1a0, ...) at conf/domain_event.c:1455
 #4  0x00007f28adc03831 in virObjectEventStateDispatchCallbacks (callbacks=<optimized out>, ....) at conf/object_event.c:724
 #5  virObjectEventStateQueueDispatch (callbacks=0x7f288c083730, queue=0x7fff51f90030, state=0x7f288c18da20) at conf/object_event.c:738
 #6  virObjectEventStateFlush (state=0x7f288c18da20) at conf/object_event.c:816
 #7  virObjectEventTimer (timer=<optimized out>, opaque=0x7f288c18da20) at conf/object_event.c:562
 #8  0x00007f28adb859cd in virEventPollDispatchTimeouts () at util/vireventpoll.c:459

Frame #0 is unrefing an invalid identity object while frame #2 hints
that the client is still dispatching the event.

For untrimmed backtrace see the bugzilla attachment.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1203030
(cherry picked from commit a98129c0)

/var/run may reside on a tmpfs and we fail to create the PID file if
/var/run/lxc does not exist.

Since commit 0a8addc1, the lxc driver's state directory isn't
automatically created before starting a domain. Now, the lxc driver
makes sure the state directory exists when it initializes.
Signed-off-by: NLubomir Rintel <lkundrak@v3.sk>
(cherry picked from commit da33a1ac)

https://bugzilla.redhat.com/show_bug.cgi?id=1199182 documents that
after a series of disk snapshots into existing destination images,
followed by active commits of the top image, it is possible for
qemu 2.2 and earlier to end up tracking a different name for the
image than what it would have had when opening the chain afresh.
That is, when starting with the chain 'a <- b <- c', the name
associated with 'b' is how it was spelled in the metadata of 'c',
but when starting with 'a', taking two snapshots into 'a <- b <- c',
then committing 'c' back into 'b', the name associated with 'b' is
now the name used when taking the first snapshot.

Sadly, older qemu doesn't know how to treat different spellings of
the same filename as identical files (it uses strcmp() instead of
checking for the same inode), which means libvirt's attempt to
commit an image using solely the names learned from qcow2 metadata
fails with a cryptic:

error: internal error: unable to execute QEMU command 'block-commit': Top image file /tmp/images/c/../b/b not found

even though the file exists.  Trying to teach libvirt the rules on
which name qemu will expect is not worth the effort (besides, we'd
have to remember it across libvirtd restarts, and track whether a
file was opened via metadata or via snapshot creation for a given
qemu process); it is easier to just always directly ask qemu what
string it expects to see in the first place.

As a safety valve, we validate that any name returned by qemu
still maps to the same local file as we have tracked it, so that
a compromised qemu cannot accidentally cause us to act on an
incorrect file.

* src/qemu/qemu_monitor.h (qemuMonitorDiskNameLookup): New
prototype.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONDiskNameLookup):
Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorDiskNameLookup): New function.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONDiskNameLookup)
(qemuMonitorJSONDiskNameLookupOne): Likewise.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit)
(qemuDomainBlockJobImpl): Use it.
Signed-off-by: NEric Blake <eblake@redhat.com>
(cherry picked from commit f9ea3d60)

Commit 4f25146b (v1.2.8) managed to silence Coverity, but at the
cost of a memory leak detected by valgrind:
==24129== 40 bytes in 5 blocks are definitely lost in loss record 355 of 637
==24129==    at 0x4A08B1C: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==24129==    by 0x5084B8E: virReallocN (viralloc.c:245)
==24129==    by 0x514D5AA: virDomainObjListExport (domain_conf.c:22200)
==24129==    by 0x201227DB: qemuConnectListAllDomains (qemu_driver.c:18042)
==24129==    by 0x51CC1B6: virConnectListAllDomains (libvirt-domain.c:6797)
==24129==    by 0x14173D: remoteDispatchConnectListAllDomains (remote.c:1580)
==24129==    by 0x121BE1: remoteDispatchConnectListAllDomainsHelper (remote_dispatch.h:1072)

In short, every time a client calls a ListAll variant and asks
for the resulting list, but there are 0 elements to return, we
end up leaking the 1-entry array that holds the NULL terminator.

What's worse, a read-only client can access these functions in a
tight loop to cause libvirtd to eventually run out of memory; and
this can be considered a denial of service attack against more
privileged clients.  Thankfully, the leak is so small (8 bytes per
call) that you would already have some other denial of service with
any guest calling the API that frequently, so an out-of-memory
crash is unlikely enough that this did not warrant a CVE.

* daemon/remote.c (remoteDispatchConnectListAllDomains)
(remoteDispatchDomainListAllSnapshots)
(remoteDispatchDomainSnapshotListAllChildren)
(remoteDispatchConnectListAllStoragePools)
(remoteDispatchStoragePoolListAllVolumes)
(remoteDispatchConnectListAllNetworks)
(remoteDispatchConnectListAllInterfaces)
(remoteDispatchConnectListAllNodeDevices)
(remoteDispatchConnectListAllNWFilters)
(remoteDispatchConnectListAllSecrets)
(remoteDispatchNetworkGetDHCPLeases): Plug leak.
Signed-off-by: NEric Blake <eblake@redhat.com>
(cherry picked from commit 3c2ff502)

Commit 4bbe1029 fixed a problem in commit f7afeddc by moving the call
to virNetDevGetIndex() to a location common to all interface types (so
that the nicindex array would be filled in for macvtap as well as tap
interfaces), but the location was *too* common, as the original call
to virNetDevGetIndex() had been in a section qualified by "if
(cfg->privileged)". The result was that the "fixed" libvirtd would try
to call virNetDevGetIndex() even for session mode libvirtd, and end up
failing with the log message:

  Unable to open control socket: Operation not permitted

To remedy that, this patch qualifies the call to virNetDevGetIndex()
in its new location with cfg->privileged.

This resolves https://bugzilla.redhat.com/show_bug.cgi?id=1198244

(cherry picked from commit 705242f8)

As reported on the libvirt-users list [1], there's new web
application called mist.io which uses libvirt as one of its
backends. Lets add it into our list of libivrt based
applications.

1: https://www.redhat.com/archives/libvirt-users/2015-February/msg00096.htmlSigned-off-by: NMichal Privoznik <mprivozn@redhat.com>

Commit cf2d4c60 used a logical or instead of bitwise or,
effectively passing 1, that is VIR_DOMAIN_XML_INACTIVE.

This was caught by a warning when building with clang.

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

Commit 69929941 started filling the listen attribute
of the parent <graphics> elements from type='network' listens.

When this XML is passed to UpdateDevice, parsing fails:
XML error: graphics listen attribute 10.20.30.40 must match
address attribute of first listen element (found none)

Ignore the address in the parent <graphics> attribute
when no type='address' listens are found,
the same we ignore the address for the <listen> subelements
when parsing inactive XML.

The gluster volume name extraction code was copied from the XML parser
without changing the VIR_ERR_XML_ERROR error code. Use
VIR_ERR_CONFIG_UNSUPPORTED instead.

Similar to commit fdb80ed4 libvirtd
would crash if a gluster URI without path would be used in the backing
chain of a volume. The crash happens in the gluster specific part of the
parser that extracts the gluster volume name from the path.

Fix the crash by checking that the PATH is NULL.

This patch does not contain a test case as it's not possible to test it
with the current infrastructure as the test suite would attempt to
contact the gluster server in the URI. I'm working on the test suite
addition but that will be post-release material.

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

In virNetworkDHCPHostDefParseXML an error is reported
when partialOkay == true, and none of ip, mac, name
were supplied.

Add the missing goto and error out in this case.

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

We already check whether the host id is valid or not, add a jump
to forbid invalid host id.
Signed-off-by: NLuyao Huang <lhuang@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Commit f7afeddc added code to report to systemd an array of interface
indexes for all tap devices used by a guest. Unfortunately it not only
didn't add code to report the ifindexes for macvtap interfaces
(interface type='direct') or the tap devices used by type='ethernet',
it ended up sending "-1" as the ifindex for each macvtap or hostdev
interface. This resulted in a failure to start any domain that had a
macvtap or hostdev interface (or actually any type other than
"network" or "bridge").

This patch does the following with the nicindexes array:

1) Modify qemuBuildInterfaceCommandLine() to only fill in the
nicindexes array if given a non-NULL pointer to an array (and modifies
the test jig calls to the function to send NULL). This is because
there are tests in the test suite that have type='ethernet' and still
have an ifname specified, but that device of course doesn't actually
exist on the test system, so attempts to call virNetDevGetIndex() will
fail.

2) Even then, only add an entry to the nicindexes array for
appropriate types, and to do so for all appropriate types ("network",
"bridge", and "direct"), but only if the ifname is known (since that
is required to call virNetDevGetIndex().

Previously this function relied on having ATTRIBUTE_NONNULL(1) in its
prototype rather than explicitly checking for a null
ifname. Unfortunately, ATTRIBUTE_NONNULL is just a hint to the
optimizer and code analyzers like Coverity, it doesn't actually check
anything at execution time, so the result was possible warnings from
Coverity, along with the possibility of null dereferences when ifname
wasn't available.

This patch removes the ATTRIBUTE_NONNULL from the prototype, and
checks ifname inside the function, logging an error if it's NULL (once
we've determined that the user really is trying to set a bandwidth).

libvirt was unconditionally calling virNetDevBandwidthClear() for
every interface (and network bridge) of a type that supported
bandwidth, whether it actually had anything set or not. This doesn't
hurt anything (unless ifname == NULL!), but is wasteful.

This patch makes sure that all calls to virNetDevBandwidthClear() are
qualified by checking that the interface really had some bandwidth
setup done, and checks for a null ifname inside
virNetDevBandwidthClear(), silently returning success if it is null
(as well as removing the ATTRIBUTE_NONNULL from that function's
prototype, since we can't guarantee that it is never null,
e.g. sometimes a type='ethernet' interface has no ifname as it is
provided on the fly by qemu).

The element wasn't declared under the interleave thus it was required
always to be first. This made it inconvenient when pasting new stuff to
the XML manually in the "wrong" place.

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

Do not use relPath, it has not been filled by virStorageFileMetadataNew.

The "virtio-mmio" is perfectly valid address type which we parse and
format correctly, but it's missing in our RNG schemas, hence editing a
domain with device having such address fails the validation.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Document that a complete device definition should be used
and a partial match can lead to the device being detached.

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

If the qemu binary on x86 does not support lsi SCSI controller,
but it supports virtio-scsi, we reject the virtio-specific attributes
for no reason.

Move the default controller assignment before the check.

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

Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

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

Soo. you've successfully started yourself a domain. And since you want
to use it on your host exclusively you are confident enough to
passthrough the host CPU model, like this:

  <cpu mode='host-passthrough'/>

Then, after a while, you want to save the domain into a file (e.g.
virsh save dom dom.save). And here comes the trouble. The file consist
of two parts: Libvirt header (containing domain XML among other
things), and qemu migration data. Now, the domain XML in the header is
formatted using special flags (VIR_DOMAIN_XML_SECURE |
VIR_DOMAIN_XML_UPDATE_CPU | VIR_DOMAIN_XML_INACTIVE |
VIR_DOMAIN_XML_MIGRATABLE).

Then, on your way back from the bar, you think of changing something
in the XML in the saved file (we have a command for it after all), say
listen address for graphics console. So you successfully type in the
command:

  virsh save-image-edit dom.save

Change all the bits, and exit the editor. But instead of success
you're left with sad error message:

  error: unsupported configuration: Target CPU model <null> does not
  match source Pentium Pro

Sigh. Digging into the code you see lines, where we check for ABI
stability. The new XML you've produced is compared with the old one
from the saved file to see if qemu ABI will break or not. Wait, what?
We are using different flags to parse the XML you've provided so we
were just lucky it worked in some cases? Yep, that's right.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Well, not that we are not formatting invalid XML, rather than not as
beautiful as we can:

  <cpu mode='host-passthrough'>
  </cpu>

If there are no children, let's use the singleton element.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Well, so far there are no variables to free, no cleanup work needed on
an error, so bare 'return -1;' after each error is just okay. But this
will change in a while.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This API joins the following two lines:

char *s = virBufferContentAndReset(buf1);
virBufferAdd(buf2, s, -1);

into one:

virBufferAddBuffer(buf2, buf1);

With one exception: there's no re-indentation applied to @buf1.
The idea is, that in general both can have different indentation
(like the test I'm adding proves)
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

In commit cc41c648 I've re-factored qemuMonitorFindBalloonObjectPath, but
missed that there is a memory leak. The "nextpath" variable is
overwritten while looping in for cycle and we have to free it before next
cycle.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

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

While the restriction doesn't have origin in any RFC, it matters
to us while constructing the dnsmasq config file (or command line
previously). For better picture, this is how the corresponding
part of network XML look like:

  <dns>
    <forwarder addr='8.8.4.4'/>
    <txt name='example' value='example value'/>
  </dns>

And this is how the config file looks like then:

  server=8.8.4.4
  txt-record=example,example value

Now we can see why there can't be any commas in the TXT name.
They are used by dnsmasq to separate @name and @value.

Funny, we have it in the documentation, but the code (which was
pushed back in 2011) didn't reflect that.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Making use of the ARCH_IS_S390 macro introduced with
e8083575Signed-off-by: NStefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>

Since s390 does not support usb the default creation of a usb controller
for a domain should not occur.

Also adjust s390 test cases by removing usb device instances since
usb devices are no longer created by default for s390 the s390
test cases need to be adjusted.
Signed-off-by: NStefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>

The description of the virsh command 'cpu-models' was written in the
wrong context (i.e. beside the domain states).
This patch moves the command description just to the cpu related
commands like 'cpu-baseline' and 'cpu-compare'.
Signed-off-by: NDaniel Hansel <daniel.hansel@linux.vnet.ibm.com>

Add configuration file for use with https://fedora.zanata.org
translation system

Well, imagine domains were running, and as the host went down, they
were managesaved. Later, after some time, the host went up again and
domains got restored. But without correct time. And depending on how
long was the host shut off, it may take some time for ntp to sync the
time too. But hey, wait a minute. We have an API just for that! So:

1) Introduce SYNC_TIME variable in libvirt-guests.sysconf to allow
users control over the new functionality
2) Call 'virsh domtime --sync $dom' in the libvirt-guests script.

Unfortunately, this is all-or-nothing approach (just like anything
else with the script). Domains are required to have configured and
running qemu-ga inside.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

The AAVMF and OVMF names were swapped. Reorder the one usage where it
matters so behavior doesn't change.