Rewrite the code to make usage of some VIR_AUTOFREE logic.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Now that we're using VIR_AUTOFREE there's quite a bit of clean up
possible for now unnecessary goto paths.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Let's make use of the auto __cleanup capabilities for virObjectUnref
consumers.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Let's make use of the auto __cleanup capabilities for VIR_FREE consumers.
In some cases adding or removing blank lines for readability.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

In preparation for VIR_AUTOFREE usage, let's remove a couple
of unused variables so that clang compilations won't fail.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Now that we're using VIR_AUTOPTR(virBitmap) there's a couple of methods
that we can clean up some now unnecessary goto paths.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Let's make use of the auto __cleanup capabilities for virBitmapPtr.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

In preparation for using auto free mechanism, change to using the
VIR_STEAL_PTR on @def to @ret and of course be sure to properly clean
up @def in cleanup.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Use the new helper when moving around the current node of the XPath
context.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Quite a few parts modify the XPath context current node to shift the
scope and allow easier queries. This also means that the node needs
to be restored afterwards.

Introduce a macro based on 'VIR_AUTOCLEAN' which adds a local structure
on the stack remembering the original node along with a function which
will make sure that the node is reset when the local structure leaves
scope.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Document that @func must take pointer to @type.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The helper function is used by the VIR_AUTOUNREF macro. Prior art is to
clear the pointer even if the variable goes out of scope.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The syntax-check should also check the new ones.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

We'd free only the first element of the vector leaking the rest.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

We don't need it as there's a separate macro for auto-freeing of string
lists.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Use of VIR_AUTOPTR and virString is confusing as it's a list and not a
single pointer. Replace it by VIR_AUTOSTRINGLIST as string lists are
basically the only sane NULL-terminated list we can have.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Similar to VIR_AUTOPTR, VIR_AUTOSTRINGLIST defines a list of strings
which will be freed if the pointer is leaving scope.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Let's make use of the auto __cleanup capabilities cleaning up any
now unnecessary goto paths.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Signed-off-by: NPeter Krempa <pkrempa@redhat.com>

* docs/news.xml: updated for release
Signed-off-by: NDaniel Veillard <veillard@redhat.com>

Mention my snapshot bug fixes, and the corresponding virsh command-line
parse tweak I added while working on the snapshot bug fixes.
Signed-off-by: NEric Blake <eblake@redhat.com>

The existing qemu snapshot code has a slight bug: if the domain
is currently pmsuspended, you can't use the _REDEFINE flag even
though the current domain state should have no bearing on being
able to recreate metadata state; and conversely, you can use the
_REDEFINE flag to create snapshot metadata claiming to be
pmsuspended as a bypass to the normal restrictions that you can't
create an original qemu snapshot in that state (the restriction
against pmsuspend is specific to qemu, rather than part of the
driver-agnostic snapshot_conf code).

Fix this by checking the snapshot state (when redefining) instead
of the domain state (which is a subset of snapshot states).

Fixes the second problem mentioned in https://bugzilla.redhat.com/1680304Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Both block_size and nb_block are unit32_t and multiplying them overflows
at 4GiB.

Moreover, the iscsi_*10_* APIs use 32bit number of blocks and thus they
can only address images up to 2TiB with 512B blocks. Let's use 64b
iscsi_*16_* APIs instead.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

When fetching LUNs from iscsi server the
virISCSIDirectReportLuns() is called. This function does some
libiscsi calls and then calls virISCSIDirectRefreshVol() over
each LUN found. It's unfortunate that the latter calls
virStoragePoolObjClearVols() as we lose all LUNs processed
in previous iterations.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>

Some of the recent entries deviated from the established
style used throughout the file, so let's fix them.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

Not exhaustive list of new features, improvements and bugfixes.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Jirka reported a bug that with every 'virsh pool-refresh' an
iscsi-direct pool would grow and grow. The problem is that
virISCSIDirectRefreshVol() only adds to def->capacity and
def->allocation but nothing clears it out to begin with.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>

For consistency with other error messages, and the fact that
the object is always called a virDomainSnapshot rather than
a mere virSnapshot, include the word "domain" in the error
message.
Suggested-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Commit 28f8dfdc (1.0.0) added a flag to virDomainGetXMLDesc, but
failed to document its effects.  And considering that the
MIGRATABLE flag has been the source of past bugs (CVE-2014-7823,
fixed in commit b1674ad5 (1.2.11), or even cf2d4c60 (1.2.13) where
flag mismatch broke virsh edit), make the wording wishy-washy
enough to discourage using the flag casually, by mentioning that
the resulting XML is more for internal use than for validation
against the schema.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Due to historical back-compat, bare 'virsh snapshot-create-as'
favors internal snapshots (but can't be used on domains with raw
storage), while 'virsh snapshot-create-as --disk-only' favors
external snapshots.  What's more, snapshots created with
--disk-only while the domain was running are marked as snapshot
state 'disk-snapshot', while snapshots created while the domain
was offline are marked as snapshot state 'shutdown' (a
'disk-snapshot' image might not be quiescent, while a 'shutdown'
snapshot always is).

But this leads to some interesting problems: if we create a
--disk-only snapshot of an offline guest, and then immediately try
to 'virsh snapshot-create --redefine' using the resulting XML to
overwrite the existing snapashot in place, things silently succeed,
but 'virsh snapshot-create --redefine --disk-only' fails with an
error message that the snapshot state is not 'disk-only'.  Worse,
if we delete the snapshot metadata first and then try to recreate
things, omitting --disk-only fails because the verification code
wants to force the default of an internal snapshot (which doesn't
work with raw disks), and using --disk-only still fails because the
snapshot XML is not 'disk-only' - making it impossible to recreate
the snapshot metadata (or to transfer it from one libvirtd host to
another).  Ideally, the presence or absence of the --disk-only
flag, and the presence or absence of an existing snapshot being
overwritten, shouldn't matter; if the XML is valid for one
situation, it should always be valid to redefine the metadata for
that snapshot.

Fix things by uniformly using virDomainSnapshotDefIsExternal()
(caching the results up front, and eliminating other 'if' clauses
now rendered redundant) when deciding whether the XML being
requested for redefinition should permit external or force internal
state capture (we got it right in only one out of three places in
the function).

See also https://bugzilla.redhat.com/1680304; this fixes the
domain-agnostic problems mentioned there, but another patch is
needed to fix further oddities with the qemu driver.  I did not
check for sure when the problems were introduced (git blame puts
some affected hunks as far back as 1.0.0), but it was definitely
been broken even before when commit 670e86bf (1.1.4) factored
redefine prep out of qemu code into the common snapshot_conf code.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Upcoming patches plan to introduce virDomainCheckpointPtr as a new
object for use in incremental backups, along with documentation on
how incremental backups differ from snapshots.  But first, we need
to rename any existing mention of a 'system checkpoint' to instead
be a 'full system snapshot', so that we aren't overloading
the term checkpoint.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The previous patch made it possible to split multiple commands by
adding newline, but not to split a long single command. The sequence
backslash-newline was being used as if it were a quoted newline
character, rather than completely elided the way the shell does.

Again, add more tests, although this time it seems more like I am
suffering from a leaning-toothpick syndrome with all the \.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

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>