Well, yet again one case of 'goto cleanup' while 'goto endjob'
was needed. Sorry.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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

Since iothread_id == 0 is an invalid value for QEMU let's point
that out specifically.  For the IOThreadDel code, the failure would
have ended up being a failure to find the IOThread ID; however, for
the IOThreadAdd code - an IOThread 0 was added and that isn't good.

It seems during many reviews/edits to the code the check for
iothread_id = 0 being invalid was lost - it could have originally
been in the API code, but requested to be moved - I cannot remember.

The comment for the function indicated that iothread_id had to be
a positive non-zero value; however, that wasn't checked - that is
a value of 0 is/was allowed by the API and was left up to the
hypervisor to reject the value.

More than likely this nuance was missed during the many "adjustments"
to the API in the review phase.

Allow 0 as an iothread_id and force the hypervisor to handle.
The qemuDomainPinIOThread API will look up the iothread_id of
0 and not find it and message that anyway.

Just like in commit 704cf06a, if virCgroup*() fails, the error is
already reported. There's no need to overwrite the error with a
generic one and possibly hiding the true root cause of the error.
Signed-off-by: NLuyao Huang <lhuang@redhat.com>

Fix inconsistency between function description and actual
parameter name in virConfGetValue/virConfSetValue.
Signed-off-by: NCao jin <caoj.fnst@cn.fujitsu.com>

When looking up a domain, we try to look up by ID, UUID and NAME
consequently while not really caring which of those lookups succeeds.
The problem is that if any of them fails, we dispatch the error from the
driver and that means setting both threadlocal and global error. Let's
say the last lookup (by NAME) succeeds and resets the threadlocal error as any
other API does, however leaving the global error unchanged. If the underlying
virsh command does not succeed afterwards, our cleanup routine in
vshCommandRun ensures that no libvirt error will be forgotten and that's
exactly where this global error comes in incorrectly.

 # virsh domif-setlink 123 vnet1 up
 error: interface (target: vnet1) not found
 error: Domain not found: no domain with matching id 123

This patch also resets the global error which would otherwise cause some
minor confusion in reported error messages.

https://bugzilla.redhat.com/show_bug.cgi?id=1254152Signed-off-by: NLuyao Huang <lhuang@redhat.com>
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Ever since commit e44b0269, 64-bit mingw compilation fails with:

../../src/util/virprocess.c: In function 'virProcessGetPids':
../../src/util/virprocess.c:628:50: error: passing argument 4 of 'virStrToLong_i' from incompatible pointer type [-Werror=incompatible-pointer-types]
         if (virStrToLong_i(ent->d_name, NULL, 10, &tmp_pid) < 0)
                                                  ^
In file included from ../../src/util/virprocess.c:59:0:
../../src/util/virstring.h:53:5: note: expected 'int *' but argument is of type 'pid_t * {aka long long int *}'
 int virStrToLong_i(char const *s,
     ^
cc1: all warnings being treated as errors

Although mingw won't be using this function, it does compile the
file, and the fix is relatively simple.

* src/util/virprocess.c (virProcessGetPids): Don't assume pid_t
fits in int.
Signed-off-by: NEric Blake <eblake@redhat.com>

It may happen that user (mistakenly) wants to rename a domain to
itself. Which is no renaming at all. We should reject that with
some meaningful error message.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

If this function fails, the error message is reported only in
some cases (e.g. OOM), but in some it's not (e.g. duplicate key).
This fact is painful and we should either not report error at all
or report the error in all possible cases. I vote for the latter.
Unfortunately, since the key may be an arbitrary value (not
necessarily a string) we can't report it in the error message.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

In 9190f0b0 we've tried to fix an OOM. And boy, was that fix
successful. But back then, the hash table implementation worked
strictly over string keys, which is not the case anymore. Hash
table have this function keyCopy() which returns void *.
Therefore a local variable that is temporarily holding the
intermediate return value from that function should be void *
too.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This function will change the guest name, we shouldn't
allow the readonly user do this.
Signed-off-by: NLuyao Huang <lhuang@redhat.com>

Signed-off-by: NLuyao Huang <lhuang@redhat.com>

Coverity complained that 'vm' wasn't initialized before jumping to
cleanup: and calling virDomainObjEndAPI if the VIR_STRDUP fails.
So I initialized vm = NULL and also moved the VIR_STRDUP closer to
usage and used endjob for goto. Lots of other reasons for failures.

That causes shadowing errors on older compilers.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

This patch removes the static keyword from the vshReadline which was
introduced in commit 834c5720. With
readline the vshReadline function is not static but when compiling
without readline it was defined as static which caused compilation
error.

In order to share as much virsh' logic as possible with upcomming
virt-admin client we need to split virsh logic into virsh specific and
client generic features.

Since majority of virsh methods should be generic enough to be used by
other clients, it's much easier to rename virsh specific data to virshX
than doing this vice versa. It moved generic virsh commands (including info
and opts structures) to generic module vsh.c.

Besides renaming methods and structures, this patch also involves introduction
of a client specific control structure being referenced as private data in the
original control structure, introduction of a new global vsh Initializer,
which currently doesn't do much, but there is a potential for added
functionality in the future.
Lastly it introduced client hooks which are especially necessary during
client connecting phase.

Currently supports only renaming inactive domains without snapshots.
Signed-off-by: NTomas Meszaros <exo@tty.sk>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This should be emitted whenever a domain is renamed.
Signed-off-by: NTomas Meszaros <exo@tty.sk>

We just need to update the entry in the second hash table. Since commit 8728a565
we have two hash tables for the domain list so that we can do O(1) lookup
regardless of looking up by UUID or name. Since with renaming a domain UUID does
not change, we only need to update the second hash table, where domains are
referenced by their name.

We will call both functions from the qemuDomainRename().
Signed-off-by: NTomas Meszaros <exo@tty.sk>

This patch implements new virsh command, domrename.

Using domrename, it will be possible to rename domain from the virsh shell by
calling virRenameDomain API.

It takes two arguments, current domain name and new domain name.

Example:

virsh # list --all
 Id    Name                           State
 ----------------------------------------------------
  -     bar                            shut off

virsh # domrename bar foo
Domain successfully renamed

virsh # list --all
 Id    Name                           State
 ----------------------------------------------------
  -     foo                            shut off

virsh #
Signed-off-by: NTomas Meszaros <exo@tty.sk>

Also, among with this new API new ACL that restricts rename
capability is invented too.
Signed-off-by: NTomas Meszaros <exo@tty.sk>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

RedHat and Debian based distros use different locations

Reference: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=790935

Otherwise the error is just

    error: Failed to create domain from test1.xml
    error: failed to retrieve file descriptor for interface: Transport endpoint is not connected

since we don't get a sensible error after the fork.

Since it's not used, let's remove it to avoid any future usage.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Pinning information returned for emulatorpin and vcpupin calls is being
returned from our data without querying cgroups for some time.  However,
not all the data were utilized.  When automatic placement is used the
information is not returned for the calls mentioned above.  Since the
numad hint in private data is properly saved/restored, we can safely use
it to return true information.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1162947Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

The numad hint stored in priv->autoNodeset is information that gets lost
during daemon restart.  And because we would like to use that
information in the future, we also need to save it in the status XML.
For the sake of tests, we need to initialize nnumaCell_max to some
value, so that the restoration doesn't fail in our test suite.  There is
no need to fill in the actual numa cell data since the recalculating
function virCapabilitiesGetCpusForNodemask() will not fail, it will just
skip filling the data in the bitmap which we don't use in tests anyway.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

This needs a reorder of XML option definitions.  It might come in handy
one day.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

When parsing private domain data, there are two paths that are flawed.
They are both error paths, just from different parts of the function.
One of them can call free() on an uninitialized pointer.  Initialization
to NULL is enough here.  The other one is a bit trickier to explain, but
as easy as the first one to fix.  We create capabilities, parse them and
then assign them into the private data pointer inside the domain object.
If, however, we get to fail from now on, the error path calls unrefs the
capabilities and then, when the domain object is being cleaned,
qemuDomainObjPrivateFree() tries to unref them as well.  That causes a
segfault.  Settin the pointer to NULL upon successful addition to the
private data is enough.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1210587  (completed)

When generating the default drive address for a SCSI <disk> device,
check the generated address to ensure it doesn't conflict with a SCSI
<hostdev> address. The <disk> address generation algorithm uses the
<target> "dev" name in order to determine which controller and unit
in order to place the device. Since a SCSI <hostdev> device doesn't
require a target device name, its placement on the guest SCSI address
"could" conflict.  For instance, if a SCSI <hostdev> exists at
controller=0 unit=0 and an attempt to hotplug 'sda' into the guest
made, there would be a conflict if the <hostdev> is already using
/dev/sda.

Create local controller/bus variables to be used by a future patch

https://bugzilla.redhat.com/show_bug.cgi?id=1210587  (partial)

If a SCSI subsystem <hostdev> element address is provided, we need to
make sure the address provided doesn't conflict with an existing or
libvirt generated address for a SCSI <disk> element. We can handle
this condition in device post processing since we're not generating an
address based on some target name - rather it's either generated based
on space or provided from the user. If the user provides one that conflicts,
then we need to disallow the change.

This will fix the issue where the domain XML provided an <address> for
the <hostdev>, but not the <disk> element where the address provided
ends up being the same address used for the <disk>. A <disk> address
is generated using it's assigned <target> 'dev' name prior to the
check/validation of the <hostdev> address value.

Hot-unplugging a disk from a guest that supports hot-unplugging generates an error
in the libvirt log when running QEMU with the "-msg timestamp=on" flag.

2015-08-06 10:48:59.945+0000: 11662: error : qemuMonitorTextDriveDel:2594 :
operation failed: deleting drive-virtio-disk4 drive failed:
2015-08-06T10:48:59.945058Z Device 'drive-virtio-disk4' not found

This error is caused because the HMP results are getting prefixed with a timestamp.
Parsing the output is not reliable with STRPREFIX as the results can be prefixed with a timestamp.

Using strstr ensures that parsing the output works whether the results are prefixed or not.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: NFrank Schreuder <fschreuder@transip.nl>

This reverts commit ede34470, which
was apparently written based on testing performed before commits
1e15be1b and 9a12b6 were pushed upstream. Once those two patches are in
place, commit ede34470 is redundant, and can even cause
incorrect/unexpected behavior when auto-assigning addresses for
virtio-net devices.

In my previous commit d7f5c889 I tried to introduce support
for inbound.floor. But the code change was incomplete. This is
the change needed to fully enable the feature.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

There's a check right at the beginning of the function that
shortcuts if the function was called over all NULL arguments.
However, this was meant just as a fool-proof check so that we
don't crash if function is used in a bad manner. Anyway, it makes
Coverity unhappy as it then thinks any of the arguments could be
NULL. Well, with the current state of the code it can't.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

It may happen that an interface don't have any bandwidth set and
a new one is to be set. In that case, @ifaceBand will be NULL.
This will cause troubles later in the code when deciding what to
do.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Commit 6983d6d2 tried to improve parseRateStr but broke the build
instead for compilers that were not able to properly introspect the for
loop indexed by the enum resulting into the following error:

virsh-domain.c: In function 'parseRateStr':
virsh-domain.c:916:13: error: 'field_name' may be used uninitialized in this function [-Werror=maybe-uninitialized]
             vshError(ctl, _("malformed %s field"), field_name);
             ^
virsh-domain.c:915:13: error: 'tmp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
         if (virStrToLong_ullp(token, NULL, 10, tmp) < 0) {
             ^
Rather than trying to fix the code, refactor the function again by
reusing virStringSplit.

If you pass <disk><serial> XML to UpdateDevice, and the original device
didn't have a <serial> block, libvirtd crashes trying to read the original
NULL serial string.

Use _NULLABLE string comparisons to avoid the crash. A couple other
properties needed the change too.

Commit e8d55172 updated the domain post-parse to automatically add
pcie-root et al for certain ARM "virt" machinetypes, but didn't update
the function qemuDomainSupportsPCI() which is called later on when we
are auto-assigning PCI addresses and default settings for the PCI
controller <model> and <target> attributes. The result was that PCI
addresses weren't assigned, and the controllers didn't have their
attribute default values set, leading to an error when the domain was
started, e.g.:

  internal error: autogenerated dmi-to-pci-bridge options not set

This patch adds the same check made in the earlier patch to
qemuDomainSupportsPCI(), so that PCI address auto-assignment and
target/model default values will be set.