And update callers to actually respect the error

Users often edit XML file stored in configuration directory
thinking of modifying a domain/network/pool/etc. Thus it is wise
to let them know they are using the wrong way and give them hint.

We already have virAsprintf, so picking a similar name helps for
seeing a similar purpose.  Furthermore, the prefix V before printf
generally implies 'va_list', even though this variant was '...', and
the old name got in the way of adding a new va_list version.

global rename performed with:

$ git grep -l virBufferVSprintf \
  | xargs -L1 sed -i 's/virBufferVSprintf/virBufferAsprintf/g'

then revert the changes in ChangeLog-old.

Suggested by Daniel P. Berrange

Make virFileBuildPath operate on the heap instead of the stack. It
allocates a buffer instead of expecting a preexisting buffer.

If pool xml has no definition for "port", then "Segmentation fault"
happens when jumping to "cleanup:" to do "VIR_FREE(port)", as "port"
was not initialized in this situation.

* src/conf/storage_conf.c

Pure cosmetic change.

VIR_ERR_INVALID_* is meant for invalid pointers only.

Making this change makes it easier to spot the memory leaks
that will be fixed in the next patch.

* cfg.mk (sc_prohibit_xmlGetProp): New rule.
* .x-sc_prohibit_xmlGetProp: New exception.
* Makefile.am (EXTRA_DIST): Ship exception file.
* tools/virsh.c (cmdDetachInterface, cmdDetachDisk): Adjust
offenders.
* src/conf/storage_conf.c (virStoragePoolDefParseSource):
Likewise.
* src/conf/network_conf.c (virNetworkDHCPRangeDefParseXML)
(virNetworkIPParseXML): Likewise.

The XML docs describe a 'port' attribute for the
storage source <host> element, but the parser never
handled it.

* docs/schemas/storagepool.rng: Define port attribute
* src/conf/storage_conf.c: Add missing parsing/formatting
  of host port number
* src/conf/storage_conf.h: Remove bogus/unused 'protocol' field

Since bugs due to double-closed file descriptors are difficult to track down in a multi-threaded system, I am introducing the VIR_CLOSE(fd) macro to help avoid mistakes here.

There are lots of places where close() is being used. In this patch I am only cleaning up usage of close() in src/conf where the problems were.

I also dare to declare close() as being deprecated in libvirt code base (HACKING).

I wrote a patch to add support for listing the Vendor and Model of a
storage pool in the storage pool XML.  This would allow vendor
extensions of specific devices.  The patch includes a test for the new
attributes as well.

Patrick Dignan

This patch fixes a couple of complaints from valgrind when tickling libvirtd with SIGHUP.

The first two files contain fixes for memory leaks. The 3rd one initializes an uninitialized variable. The 4th one is another memory leak.

* Use virBufferEscapeString() per Dan B.

The storage pool driver is not doing correct checking for
duplicate UUID/name values. This introduces a new method
virStoragePoolObjIsDuplicate, based on the previously
written virDomainObjIsDuplicate.

* src/conf/storage_conf.c, src/conf/storage_conf.c,
  src/libvirt_private.syms: Add virStoragePoolObjIsDuplicate,
* src/storage/storage_driver.c: Call virStoragePoolObjIsDuplicate
  for checking uniqueness of uuid/names

Spurious / in a pool target path makes life difficult for apps using the
GetVolByPath, and doing other path based comparisons with pools. This
has caused a few issues for virt-manager users:

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

Add a new util API which removes spurious /, virFileSanitizePath. Sanitize
target paths when parsing pool XML, and for paths passed to GetVolByPath.

v2: Leading // must be preserved, properly sanitize path=/, sanitize
    away /./ -> /

v3: Properly handle starting ./ and ending /.

v4: Drop all '.' handling, just sanitize / for now.

* cfg.mk (useless_free_options): Add many vir*Free* function names,
and then remove the useless if-before-free tests exposed by running
make syntax-check.
* src/conf/interface_conf.c (virInterfaceDefFree): Remove useless "if".
(virInterfaceAssignDef): Likewise.
* src/conf/network_conf.c (virNetworkAssignDef): Likewise.
* src/conf/storage_conf.c (virStoragePoolObjAssignDef): Likewise.
* src/node_device/node_device_hal.c (dev_create): Likewise.
* src/security/virt-aa-helper.c (vahDeinit): Likewise.
* src/test/test_driver.c (testNodeDeviceCreateXML): Likewise.
* src/util/conf.c (virConfSetValue): Likewise.

ESX supports NFS and CIFS. The ESX storage driver will reflect this.

* virStorageEncryptionFormat is called from both
  virDomainDiskDefFormat and virStorageVolTargetDefFormat.  The proper
  indentation in the generated XML depends on the caller.  My earlier
  patch to fix the incorrect indentation for the domain XML broke the
  indentation for the storage XML.  This patch adopts Laine's
  suggestion of requring the caller of virStorageEncryptionFormat to
  provide an unsigned int with the number of spaces the output should
  be indented.  The patch modifies both callers to provide the
  additional argument.

* Add a regression test for the domain XML

* src/conf/domain_conf.c src/conf/storage_conf.c
  src/conf/storage_encryption_conf.c src/conf/storage_encryption_conf.h:
  change the indentation code
* tests/qemuxml2xmltest.c
  tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.args
  tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.xml: add a regression test

No semantic change, the now explicitly set default are all zero and
that's what GCC sets unspecified struct members to.

This allows the config to have a setting that means "leave it alone",
eg when building a pool where the directory already exists the user
may want the current uid/gid of the directory left intact. This
actually gets us back to older behavior - before recent changes to the
pool building code, we weren't as insistent about honoring the uid/gid
settings in the XML, and virt-manager was taking advantage of this
behavior.

As a side benefit, removing calls to getuid/getgid from the XML
parsing functions also seems like a good idea. And having a default
that is different from a common/useful value (0 == root) is a good
thing in general, as it removes ambiguity from decisions (at least one
place in the code was checking for (perms.uid == 0) to see if a
special uid was requested).

Note that this will only affect newly created pools and volumes. Due
to the way that the XML is parsed, then formatted for newly created
volumes, all existing pools/volumes already have an explicit uid and
gid set.

src/conf/storage_conf.c: Remove calls to setuid/setgid for default values
                         of uid/gid, and set them to -1 instead

src/storage/storage_backend.c:
src/storage/storage_backend_fs.c:
        Make account for the new default values of perms.uid
        and perms.gid.

The storage backend implementations all presume that the XML parser
is validating correctness of the source specification. The check for
a source device was lost at some point. This allowed for a potential
crash in the disk backend. Re-introduce the sanity check

* src/conf/storage_conf.c: Re-add check for source device

We were using 'Y' to mean exabyte, when the correct abbreviation would be
'E' ('Y' is yettabyte, which is exabyte * 1024 * 1024). While it isn't
strictly backwards compatible, I highly doubt anyone was actually using
this broken behavior, so I don't see any harm in in dropping 'Y' handling.

* src/conf/storage_conf.c (virStoragePoolDefParseSourceString):
* src/storage/storage_backend_fs.c:
(virStorageBackendFileSystemNetFindPoolSourcesFunc):
(virStorageBackendFileSystemNetFindPoolSources):
* src/test/test_driver.c (testStorageFindPoolSources):

Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

The virConnectPtr is no longer required for error reporting since
that is recorded in a thread local. Remove use of virConnectPtr
from all APIs in storage_conf.{h,c} and storage_encryption_conf.{h,c}
and update all callers to match

It was used for error reporting only.

* src/conf/storage_conf.c (virStoragePoolSourceListNewSource):
Remove an unused (and leaked) allocation.

Allows the initiator to use a variety of IQNs rather than just the
system IQN when creating iSCSI pools.
* docs/schemas/storagepool.rng: extends the syntax with <iqn name="..."/>
* src/conf/storage_conf.[ch]: read and stores the iqn name
* src/storage/storage_backend_iscsi.[ch]: implement the IQN selection
  when detected

* src/conf/storage_conf.c (virStoragePoolSourceListNewSource):
Free just-allocated "source" upon VIR_REALLOC_N failure.

Replace free(virBufferContentAndReset()) with virBufferFreeAndReset().
Update documentation and replace all remaining calls to free() with
calls to VIR_FREE(). Also add missing calls to virBufferFreeAndReset()
and virReportOOMError() in OOM error cases.

- Don't duplicate SystemError
- Use proper error code in domain_conf
- Fix a broken error call in qemu_conf
- Don't use VIR_ERR_ERROR in security driver (isn't a valid code in this case)

This will simplify adding FindPoolSources support to more pool backends in
the future (as well as the test driver).

Similar in theory to *AssignDef type functions, this duplicate functionality
will be used by an future FindPoolSources implementations.

We need to parse a source XML block for FindPoolSources, so this is a step
in sharing the parsing. The new storage pool XML 2 XML tests cover this area
pretty well to ensure we aren't causing regressions.

* src/conf/storage_conf.c src/conf/storage_conf.h: extend the enums
  and values
* docs/schemas/storagepool.rng: add to the list of storage pool type
  formats

We were missing a closing tag, so the XML wasn't proper.