A single PCI device may have multiple netdevs associated with it. Each
of those netdevs will have a different phys_port_id entry in
sysfs. This patch modifies virPCIGetNetName() to allow selecting one
of the potential many netdevs in two different ways:

1) by setting the "idx" argument, the caller can select the 1st (0),
2nd (1), etc. netdev from the PCI device's net subdirectory.

2) If the physPortID arg is set (to a null-terminated string) then
virPCIGetNetName() returns the netdev that has that phys_port_id in
the sysfs file of the same name in the netdev's directory.

The first arg isn't modified in the function, so it should be const.

On Linux each network device *can* (but not necessarily *does*) have
an attribute called phys_port_id which can be read from the file of
that name in the netdev's sysfs directory. The examples I've seen have
been a many-digit hexadecimal number (as an ASCII string).

This value can be useful when a single PCI device is associated with
multiple netdevs (e.g a dual port Mellanox SR-IOV NIC - this card has
a single PCI Physical Function (PF), and that PF has two netdevs
associated with it (the "net" subdirectory of the PF in sysfs has two
links rather than the usual single link to a netdev directory). Each
of the PF netdevs has a different phys_port_id. The Virtual Functions
(VF) are similar - the PF (a PCI device) has "n" VFs (also each of
these is a PCI device), each VF has two netdevs, and each of the VF
netdevs points back to the VF PCI device (with the "device" entry in
its sysfs directory) as well as having a phys_port_id matching the PF
netdev it is associated with.

virNetDevGetPhysPortID() simply attempts to read the phys_port_id for
the given netdev and return it to the caller. If this particular
netdev driver doesn't support phys_port_id, it returns NULL (*not* a
NULL-terminated string, but a NULL pointer) but still counts it as a
success.

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

This code is so complicated because we allow enabling the same
bits at many places. Just like in this case: huge pages can be
enabled by global <hugepages/> element under <memoryBacking> or
on per <memory/> basis. To complicate things a bit more, users
are allowed to omit the page size which case the default page
size is used. And this is what is causing this bug. If no page
size is specified, @pagesize is keeping value of zero throughout
whole function. Therefore we need yet another boolean to hold
[use, don't use] information as we can't sue @pagesize for that.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.com>

In virDomainNetDefParseXML() the def->coalesce is parsed and
allocated by virDomainNetDefCoalesceParseXML() but in fact it's
never freed .
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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

Currently, there's a bug when undefining a domain with NVRAM
store. Basically, the unlink() of the NVRAM store file happens
during the undefine procedure iff domain is inactive. So, if
domain is running and undefine is called the file is left behind.
It won't be removed in the domain cleanup process either
(qemuProcessStop). One of the solutions is to remove if
regardless of the domain state and rely on qemu having the file
opened. This still has a downside that if the domain is defined
back the NVRAM store file is going to be new, any changes to the
current one are lost (just like with any other file that is
deleted while a process has it opened). But is it really a
downside?
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Rewrite virConfReadMem to take a null-terminated string.
All the callers were calling strlen on it anyway.

Make the comparison explicit.

Slight refactor of the WMI serialization code to minimize mixing
openwsman and libxml2 APIs that triggered clang alignment warnings.

The only usage of libxml2 APIs now is in creating CDATA blocks,
because the openwsman API does not provide that functionality. The
clang alignment warning in this case is silenced by casting to a
void pointer first.

The API docs for the various vir$OBJECTGetConnect functions
contain a warning

  WARNING: When writing libvirt bindings in other languages, do
  not use this function.  Instead, store the connection and
  the domain object together.

There is no reason why language bindings should not use this
method, and indeed the Perl, Python, and Go bindings all use
these methods.

This warning was originally added back in

  commit 3edb4bc9
  Author: Daniel Veillard <veillard@redhat.com>
  Date:   Tue Jul 24 15:32:55 2007 +0000

    * libvirt.spec.in NEWS docs/* po/*: preparing release 0.3.1
    * src/libvirt.c python/generator.py: some cleanup and warnings
      from Richard W.M. Jones

IIUC, the rational was that these APIs do not need to be
directly exposed to the non-C language, as the language
can expose the same concept itself by storing the original
virConnectPtr object alongside the virDomainPtr.  There's
no reason to mandate such an approach though - it is valid
for languages to expose this directly if that suits their
needs better.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

We're storing the machine name in @priv but free it just in
qemuProcessStop, Therefore this may leak.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Again, we are using @cssid, @ssid and @devno to store some
temporary strings, but never free it.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

When parsing <ioapic> feature we're using @tmp to store some
temporary string but never free it.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

When parsing boot options from domain XML in
virDomainDefParseBootOptions() initenv id stored to:

    def->os.initenv[i]->name
    def->os.initenv[i]->value

But these are never freed.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This patch adds support for automatic VNC port assignment for bhyve guests.
Signed-off-by: NRoman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Most other top level objects have already had their limits increased
to 16384. Increase the storage pool, nwfilter & snapshot object
limits to match. For snapshots at least, we have seen hosts which
exceeded the current limit
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

CPUID leaf 7 is sub-leaf aware. Add missing attribute.
Signed-off-by: NMarek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

The code only currently handles writing an x86 default -cpu
argument, and doesn't know anything about other architectures.
Let's make this explicit rather than leaving ex. qemu ppc64 to
throw an error about -cpu qemu64
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

Certain XML features that aren't in the <cpu> block map to -cpu
flags on the qemu cli. If one of these is specified but the user
didn't explicitly pass an XML <cpu> model, we need to format a
default model on the command line.

The current code handles this by sprinkling this default cpu handling
among all the different flag string formatting. Instead, switch it
to do this just once.

This alters some test output slightly: the previous code would
write the default -cpu in some cases when no flags were actually
added, so the output was redundant.
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

My commit 0c1d8632 broke formatting of passthrough smartcard devices:
<smartcard mode='passthrough' type='spicevmc'/>

resulted in invalid XML:
    <smartcard mode='passthrough'>
       type='spicevmc'>
      <address type='ccid' controller='0' slot='0'/>
    </smartcard>

Split out chardev source formatting function into two -
one formatting the attributes and other formatting the subelements.
Reported-by: NCole Robinson <crobinso@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Export the function from the util module so that dynamic linking can
override it.

The functions declared in virhash.h return bool, but stdbool.h was not
included.

Disk serial schema has extra '.+' allowed characters in comparison
with check in code. Looks like there is no reason for that as qemu
allows any character AFAIK for serial. This discrepancy is originated
in commit id '85d15b51' where the ability to add serial was added.

Alter the disk-serial test to add a disk with all the possible
characters listed as the serial value.

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

Introduce virQEMUDriverConfigTLSDirResetDefaults in order to check
if the defaultTLSx509certdir was changed, then change the default
for any other *TLSx509certdir that was not set to the default default.

Introduce virQEMUDriverConfigValidate to validate the existence of
any of the *_tls_x509_cert_dir values that were uncommented/set,
incuding the default.

Update the qemu.conf description for default to describe the consequences
if the default directory path does not exist.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

The rombar attribute was already validated at the time of parsing
the XML.

After an OOM error, virBuffer* APIs set buf->use to zero.
Adding a buffer to the parent buffer only if use is non-zero
would quietly drop data on error.

Check the error beforehand to make sure buf->use is zero
because we have not attempted to add anything to it.

Switch virDomainHubDefFormat to use a separate buffer for subelements.

Convert virDomainWatchdogDefFormat to use a separate
buffer for subelements.

Convert virDomainSoundDefFormat to use a separate buffer
for subelements.

Convert virDomainSmartcardDefFormat to use a separate buffer
for possible subelements, to avoid the need for duplicated
formatting logic in virDomainDeviceInfoNeedsFormat.

This function has grown to format more than just the address.
Delete the comment completely to avoid failing to update it
in the future.

Also, the indentation is now handled by the virBuffer APIs,
so the comment about indentation no longer makes sense.

This function returns false if virDomainDeviceInfoFormat
would not format anything.

Using it as the sole condition to decide whether to call
virDomainDeviceInfoFormat or not is pointless, since
the conditions are repeated in virDomainDeviceInfoFormat.

This will simplify the cleanup when we start checking for
buffer errors.

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

Just like I've added support for setting rx_queue_size (in
c56cdf25 and friends), qemu just gained support for setting tx
ring size.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Not every platform is guaranteed to have dlopen/dlsym, so we should
conditionalize its use. Suprisingly it is actually present for Win32
via the mingw-dlfcn add on, but we should still conditionalize it.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

These functions were made exportable back in 3aa3e072 when I was
splitting network code into parsing and list management parts.
Since then the split is finished now and these two functions do
not need to be exported anymore.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This member allows us to store a pointer to some private data.
However, the comment says it's used in both domain driver and
network driver. Well, it is not. It's just one pointer and domain
driver uses it directly. Network driver has a global driver
variable. Update the comment to not confuse others.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Since the introduction of shmem, there was a split of preparation code
from the formatting code from qemuBuildCommandLine() into
qemuProcessPrepareDomain().  Let's fix shmem in this regard, so that
we can slowly get to a cleaner codebase.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

So there are couple of issues here. Firstly, we never unref the
@pendingReply and thus it leaks.

==13279== 144 (72 direct, 72 indirect) bytes in 1 blocks are definitely lost in loss record 1,095 of 1,259
==13279==    at 0x4C2E080: calloc (vg_replace_malloc.c:711)
==13279==    by 0x781FA97: _dbus_pending_call_new_unlocked (in /usr/lib64/libdbus-1.so.3.14.11)
==13279==    by 0x7812A4C: dbus_connection_send_with_reply (in /usr/lib64/libdbus-1.so.3.14.11)
==13279==    by 0x56BEDF3: virNetDaemonCallInhibit (virnetdaemon.c:514)
==13279==    by 0x56BEF18: virNetDaemonAddShutdownInhibition (virnetdaemon.c:536)
==13279==    by 0x12473B: daemonInhibitCallback (libvirtd.c:742)
==13279==    by 0x1249BD: daemonRunStateInit (libvirtd.c:823)
==13279==    by 0x554FBCF: virThreadHelper (virthread.c:206)
==13279==    by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.so)
==13279==    by 0x928DE3C: clone (in /lib64/libc-2.23.so)

Secondly, while we send the message, we are suspended ('cos we're
talking to a UNIX socket).  However, until we are resumed back
again the reply might have came therefore subsequent
dbus_pending_call_set_notify() has no effect and in fact the
virNetDaemonGotInhibitReply() callback is never called. Thirdly,
the dbus_connection_send_with_reply() has really stupid policy
for return values. To cite the man page:

  Returns
      FALSE if no memory, TRUE otherwise.

Yes, that's right. If anything goes wrong and it's not case of
OOM then TRUE is returned, i.e. you're trying to pass FDs and
it's not supported, or you're not connected, or anything else.
Therefore, checking for return value of
dbus_connection_send_with_reply() is not enoguh. We also have to
check if @pendingReply is not NULL before proceeding any further.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>