Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

If a domain object is being removed and looked up concurrently we must
ensure we unlock the object before unreferencing it, since the latter
might free the object.

The flaw was introduced in commit feb1a4d7.
Signed-off-by: NMichael Chapman <mike@very.puzzling.org>

Undefining a running, autostarted domain removes the autostart link, but
dom->autostart is not cleared. If the domain is subsequently redefined,
libvirt thinks it is already autostarted and will not create the link
even if requested:

  # virsh dominfo example | grep Autostart
  Autostart:      enable

  # ls /etc/libvirt/qemu/autostart/example.xml
  /etc/libvirt/qemu/autostart/example.xml

  # virsh undefine example
  Domain example has been undefined

  # virsh define example.xml
  Domain example defined from example.xml

  # virsh dominfo example | grep Autostart
  Autostart:      enable

  # virsh autostart example
  Domain example marked as autostarted

  # ls /etc/libvirt/qemu/autostart/example.xml
  ls: cannot access /etc/libvirt/qemu/autostart/example.xml: No such file or directory

This commit ensures dom->autostart is cleared whenever the config and
autostart link (if present) are removed.

The bridge network driver cleared this flag itself in networkUndefine.
This commit moves this into virNetworkDeleteConfig for symmetry with
virDomainDeleteConfig, and to ensure it is not missed in future network
drivers.
Signed-off-by: NMichael Chapman <mike@very.puzzling.org>

Now that all the code uses accessors, don't expose the structure
anyway.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Well, one day this will be self-locking object, but not today.
But lets prepare the code for that! Moreover,
virNetworkObjListFree() is no longer needed, so turn it into
virNetworkObjListDispose().
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

The API will iterate over the list of network object and remove
desired ones from it.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

An accessor following pattern laid out by virDomainObjList* APIs.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

An accessor following pattern laid out by virDomainObjList* APIs.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This API will be used in the future to call passed callback over
each network object in the list. It's slightly different to its
virDomainObjListForEach counterpart, because virDomainObjList
uses a hash table to store domain object, while virNetworkObjList
uses an array.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

We have something like pvpanic device. However, in some cases it does
not have any address assigned, in which case we produce this ugly XML
(still valid though):

  <devices>
    <emulator>/usr/bin/qemu</emulator>
    ...
    <panic>
    </panic>
  </devices>

Lets format "<panic/>" instead.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

There was a mess in the way how we store unlimited value for memory
limits and how we handled values provided by user.  Internally there
were two possible ways how to store unlimited value: as 0 value or as
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED.  Because we chose to store memory
limits as unsigned long long, we cannot use -1 to represent unlimited.
It's much easier for us to say that everything greater than
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED means unlimited and leave 0 as valid
value despite that it makes no sense to set limit to 0.

Remove unnecessary function virCompareLimitUlong.  The update of test
is to prevent the 0 to be miss-used as unlimited in future.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1146539Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

Since the APIs support just one element per namespace and while
modifying an element all duplicates would be removed, let's do this
right away in the post parse callback.

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

Adding functionality to libvirt that will allow it
query the ethtool interface for the availability
of certain NIC HW offload features

Here is an example of the feature XML definition:

<device>
<name>net_eth4_90_e2_ba_5e_a5_45</name>
  <path>/sys/devices/pci0000:00/0000:00:03.0/0000:08:00.1/net/eth4</path>
  <parent>pci_0000_08_00_1</parent>
  <capability type='net'>
    <interface>eth4</interface>
    <address>90:e2:ba:5e:a5:45</address>
    <link speed='10000' state='up'/>
    <feature name='rx'/>
    <feature name='tx'/>
    <feature name='sg'/>
    <feature name='tso'/>
    <feature name='gso'/>
    <feature name='gro'/>
    <feature name='rxvlan'/>
    <feature name='txvlan'/>
    <feature name='rxhash'/>
    <capability type='80203'/>
  </capability>
</device>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

It's returning virNetworkObjPtr after all. And it matches the
pattern laid out by domain_conf.h.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

It's returning virNetworkObjPtr after all. And it matches the
pattern laid out by domain_conf.h.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Instead of copying the whole object onto stack when calling the
function, just pass the pointer to the object and save up some
space on the stack. Moreover, this prepares the code to hide the
virNetworkObjList structure into network_conf.c and use accessors
only.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

All of our vir*Free() functions should accept NULL, even though
that there's no way of actually passing NULL with current code.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This is probably a copy-paste error from virDomainObj*
counterpart.  But when speaking of virNetworkObj we should use
variable @nets for an array of networks, rather than @doms. It's
just confusing.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Since adding the support for scheduler policy settings in commit
8680ea97, there are two enums with the same information.  That was
caused by rewriting the patch since first draft.

Find out thanks to clang, but there was no impact whatsoever.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

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

This patch resolves a situation where the same "<target dev='$name'...>"
can be used for multiple disks in the domain.

While the $name is "mostly" advisory regarding the expected order that
the disk is added to the domain and not guaranteed to map to the device
name in the guest OS, it still should be unique enough such that other
domblk* type operations can be performed.

Without the patch, the domblklist will list the same Target twice:

$ virsh domblklist $dom
Target     Source
------------------------------------------------
sda        /var/lib/libvirt/images/file.qcow2
sda        /var/lib/libvirt/images/file.img

Additionally, getting domblkstat, domblkerror, domblkinfo, and other block*
type calls will not be able to reference the second target.

Fortunately, hotplug disallows adding a "third" sda value:

$ qemu-img create -f raw /var/lib/libvirt/images/file2.img 10M
$ virsh attach-disk $dom /var/lib/libvirt/images/file2.img sda
error: Failed to attach disk
error: operation failed: target sda already exists

$

BUT, it since 'sdb' doesn't exist one would get the following on the same
hotplug attempt, but changing to use 'sdb' instead of 'sda'

$ virsh attach-disk $dom /var/lib/libvirt/images/file2.img sdb
error: Failed to attach disk
error: internal error: unable to execute QEMU command 'device_add': Duplicate ID 'scsi0-0-1' for device

$

Since we cannot fix this issue at parsing time, the best that can be done so
as to not "lose" a domain is to make the check prior to starting the guest
with the results as follows:

$ virsh start $dom
error: Failed to start domain $dom
error: XML error: target 'sda' duplicated for disk sources '/var/lib/libvirt/images/file.qcow2' and '/var/lib/libvirt/images/file.img'

$

Running 'make check' found a few more instances in the tests where this
duplicated target dev value was being used. These also exhibited some
duplicated 'id=' values (negating the uniqueness argument of aliases) in
the corresponding .args file and of course the *xmlout version of a few
input XML files.

The total NUMA memory consists of the sum of individual NUMA node memory
amounts.

Add VIR_VOL_XML_PARSE_OPT_CAPACITY flag to virStorageVolDefParseXML.
With this flag, no error is reported when the capacity is missing
if there is a backing store.

Add VIR_VOL_XML_PARSE_NO_CAPACITY flag to the volume XML
parser. When set, it allows the capacity element to be omitted.

So we can allow omitting the capacity element if backing store is
present.

Allow the callers to pass down libvirt-internal flags.

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.

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>

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).

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>

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>

At least Xen supports backend drivers in another domain (aka "driver
domain"). This patch introduces an XML config option for specifying the
backend domain name for <disk> and <interface> devices.  E.g.

  <disk>
    <backenddomain name='diskvm'/>
    ...
  </disk>
  <interface type='bridge'>
    <backenddomain name='netvm'/>
    ...
  </interface>

In the future, same option will be needed for USB devices (hostdev
objects), but for now libxl doesn't have support for PVUSB.
Signed-off-by: NMarek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

The function that parses the <forward> subelement of a network used to
fail/log an error if the network definition contained both a <pf>
element as well as at least one <interface> or <address> element. That
check was present because the configuration of a network should have
either one <pf>, one or more <interface>, or one or more <address>,
but never combinations of multiple kinds.

This caused a problem when libvirtd was restarted with a network
already active - when a network with a <pf> element is started, the
referenced PF (Physical Function of an SRIOV-capable network card) is
checked for VFs (Virtual Functions), and the <forward> is filled in
with a list of all VFs for that PF either in the form of their PCI
addresses (a list of <address>) or their netdev names (a list of
<interface>); the <pf> element is not removed though. When libvirtd is
restarted, it parses the network status and finds both the original
<pf> from the config, as well as the list of either <address> or
<interface>, fails the parse, and the network is not added to the
active list. This failure is often obscured because the network is
marked as autostart so libvirt immediately restarts it.

It seems odd to me that <interface> and <address> are stored in the
same array rather than keeping two separate arrays, and having
separate arrays would have made the check much simpler. However,
changing to use two separate arrays would have required changes in
more places, potentially creating more conflicts and (more
importantly) more possible regressions in the event of a backport, so
I chose to keep the existing data structure in order to localize the
change.

It appears that this problem has been in the code ever since support
for <pf> was added (0.9.10), but until commit
34cc3b2f (first in libvirt 1.2.4)
networks with interface pools were not properly marked as active on
restart anyway, so there is no point in backporting this patch any
further than that.

Later patches will need to access the full definition to do check the
memory size and thus the checking needs to be done after the whole
definition including devices is known.

Add helper to compare initial sizes of indivitual NUMA nodes and the map
of belonging vCPUs. Other configuration is not ABI.

The backing of the vm's memory isn't influencing the guest ABI thus
shouldn't be checked.

For historical reasons data regarding NUMA configuration were split
between the CPU definition and numatune. We cannot do anything about the
XML still being split, but we certainly can at least store the relevant
data in one place.

This patch moves the NUMA stuff to the right place.

Add the helpers and refactor places where the value is accessed without
them.