Add the required fields that are missing from the new structure that
will allow us to switch the storage file metadata code entirely to the
new structure.
Add "relPath" and "relDir" and the raw backing store name. Also allow
creating linked lists of virStorageSourcePtrs to express backing chains.
Remove the obsolete field replaced by data in "path".
The testsuite requires tweaking as the name of the backing file is now
stored one layer deeper in the backing chain linked list.
As a temporary step to allow killing of the "backingStore" field of
struct virStorageFileMetadata the recursive metadata retrieval function
will be converted not to use the field in the lookup process.
As for the previous patch, this change is needed to achieve
compatibility with all the existing code, where we expect a fully
qualified path of local files to be present.
To allow future change of virStorageFileMetadata to virStorageSource we
need to store a full path in the "path" variable as rest of the code
expects it to be a full path. Rename the "path" field to "relPath" to
keep tracking the info but allowing a real "path" field.
Avoid passing lot of arguments into guts of metadata retrieval to fill
the actual structure. Temporarily fill the structure before passing it
down to the actual metadata extractor.
This will later help the inversion of the steps taken to extract the
metadata so that this function can be fully converted to
virStorageSource as the data struct.
This patch also fixes regression when starting a gluster storage pool
where the volumes don't have local representation so that the
canonicalization of the volume's file name failed. Broken by commit
79f11b35
Move the code checking the presence of the backing file to the recursive
worker function instead of the metadata parser. The recursive worker
will later be changed to parse more than just local files and this
change will help the separation.
As we already pass the whole structure down the call path there's no
need to return some stuff in a separate argument. Remove the argument
and tweak callers to avoid breaking semantics.
virStorageFileGetMetadataFromBuf will be refactored later along with the
storage driver.
Don't use the backingStoreRaw as a indication of broken chains. Fill it
always and tweak the broken image chain detector to avoid changing the
semantics.
The new semantics to detect a broken chain is the presence of string in
backingStoreRaw but the lack of the backing chain metadata structure in
the chain.
Now that the raw backing store name is always filled there's no need to
pass the raw name variable separately to fill in case the backing is not
a file. Tweak the function so that it can handle a NULL in that case.
- do not lose new definition for an active domain
- do not leak oldDef
- do not set dom->id if virDomainSaveConfig() fails
- do not call virObjectUnlock(vm) if vm is NULL
Currently, libvirt is using legacy USB controller as default. There
are problems with VGA which can't work correctly with USB Keyboard and
USB Mouse.
While providing -nodefaults, ppc64 should be specifying the usb
controller explicitly in place of using the legacy
controller(-usb). Qemu spapr initialization code when sees "-usb" adds a
USB Keyboard and USB Mouse by default. And libvirt has added a USB
keyboard and USB mouse.
A recent fix in the in qemu VGA code uncoverd this problem, which
resulted in addition of extra keyboard and mouse to the qemu machine.
This patch is to set pci-ohci as USB default controller.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com>
Remove the pointer from def->cputune.vcpupin after unplugging
the CPU and also free the bitmap contained in the structure
by calling virDomainVcpuPinDel instead of VIR_FREE.
Introduced by commit 0df1a79.
This makes virDomainLookupVcpuPin redundant.
https://bugzilla.redhat.com/show_bug.cgi?id=1088165
Implement bhyveDomainCreateXML function.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
While running virstoragetest, valgrind pointed out the following
memory leak:
==8142== 2 bytes in 1 blocks are definitely lost in loss record 1 of 92
==8142== at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==8142== by 0x4E7B53E: mdir_name (dirname-lgpl.c:78)
==8142== by 0x4CBE2B0: virStorageFileGetMetadataInternal (virstoragefile.c:595)
==8142== by 0x4CBE651: virStorageFileGetMetadataFromFDInternal (virstoragefile.c:1086)
==8142== by 0x4CBEEB4: virStorageFileGetMetadataRecurse (virstoragefile.c:1175)
==8142== by 0x4CBF1DE: virStorageFileGetMetadata (virstoragefile.c:1270)
==8142== by 0x4028AD: testStorageChain (virstoragetest.c:275)
==8142== by 0x407B91: virtTestRun (testutils.c:201)
==8142== by 0x4039D7: mymain (virstoragetest.c:534)
==8142== by 0x40830D: virtTestMain (testutils.c:789)
==8142== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
...62 times
Commit id '18642d10' caused a virt-test regression for NFS backend
storage error path checks when running the command:
'virsh find-storage-pool-sources-as netfs Unknown '
when the host did not have Gluster installed. Prior to the commit,
the test would fail with the error:
error: internal error: Child process (/usr/sbin/showmount --no-headers
--exports Unknown) unexpected exit status 1: clnt_create: RPC: Unknown host
After the commit, the error would be ignored, the call would succeed,
and an empty list of pool sources returned. This was tucked into the
commit message as an expected outcome.
When the target host does not have a GLUSTER_CLI this is a regression
over the previous release. Furthermore, even if Gluster CLI was present,
but had a failure to get devices, the API would return a failure even if
the NFS backend had found devices.
Modify the logic to return failure when the NFS backend check fails and
there's no GLUSTER_CLI or when both backend checks fail.
If either returns success and GLUSTER_CLI is defined, then fetch and return
a list of source devices even if it's empty
I noticed that depending on the <driver> attributes the user passed
in, the output may omit the <driver> element altogether. For example,
the rerror_policy has had this problem since commit 4bb4109 in Oct
2011. But in adding testsuite coverage to expose it, I found another
problem: the C code is just fine without a driver name, but the
XML validator required either a name or a cache mode.
* src/conf/domain_conf.c (virDomainDiskDefFormat): Update
conditional.
* docs/schemas/domaincommon.rng (diskDriver): Simplify.
* tests/qemuxml2argvdata/qemuxml2argv-disk-drive-copy-on-read.xml:
* tests/qemuxml2argvdata/qemuxml2argv-disk-drive-copy-on-read.args:
New files.
* tests/qemuxml2argvdata/qemuxml2argv-disk-drive-discard.xml:
Enhance test.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-discard.xml:
Likewise.
* tests/qemuxml2argvtest.c (mymain): New test.
* tests/qemuxml2xmltest.c (mymain): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Domain snapshots should only permit an external snapshot into
a storage format that permits a backing chain, since the new
snapshot file necessarily must be backed by the existing file.
The C code for the qemu driver is a little bit stricter in
currently enforcing only qcow2 or qed, but at the XML parser
level, including virt-xml-validate, it is fairly easy to
enforce that a user can't request a 'raw' external snapshot.
* docs/schemas/storagecommon.rng (storageFormat): Split out...
(storageFormatBacking): ...new sublist.
* docs/schemas/domainsnapshot.rng (disksnapshotdriver): Use new
type.
* src/util/virstoragefile.h (virStorageFileFormat): Rearrange for
easier code management.
* src/util/virstoragefile.c (virStorageFileFormat, fileTypeInfo):
Likewise.
* src/conf/snapshot_conf.c (virDomainSnapshotDiskDefParseXML): Use
new marker to limit selection of formats.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Commit 4897698 fixed the build without dbus by only building
the virSystemdPMSupportTarget with SYSTEMD_DAEMON.
Introduce a virDBusMessageUnref wrapper for dbus_message_unref
to let virsystemd.c build without dbus, while still allowing
virsystemdtest to run without SYSTEM_DAEMON.
In order to do that, virNodeSuspendSupportsTargetPMUtils() and
virSystemdPMSupportTarget() are created even when pm-utils and dbus
are compiled out, respectively, but in that case returning -2 meaning
"unavailable" (this return code was already used for unavailability
before). Error is reported in virNodeSuspendSupportsTarget() only if
both functions returned -2, otherwise the error (or success) is properly
propagated up the stack.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
When passing migration bandwidth to QEMU, we multiply it by 1024 * 1024
to convert the speed to B/s and the result still needs to fit in
int64_t.
https://bugzilla.redhat.com/show_bug.cgi?id=1083483
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Another field no longer needed, getting us one step closer to
merging virStorageFileMetadata and virStorageSource.
* src/util/virstoragefile.h (_virStorageFileMetadata): Drop
field.
* src/util/virstoragefile.c (virStorageFileGetMetadataInternal)
(virStorageFileGetMetadataFromFDInternal): Alter signature.
(virStorageFileFreeMetadata, virStorageFileGetMetadataFromBuf)
(virStorageFileGetMetadataFromFD): Adjust clients.
* tests/virstoragetest.c (_testFileData, testStorageChain)
(mymain): Simplify test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Thanks to the testsuite, I feel quite confident that this rewrite
is correct; it gives the same results for all cases except for one.
I can make the argument that _that_ case was a pre-existing bug:
when looking up relative names, the lookup is supposed to be
pegged to the directory that contains the parent qcow2 file. Thus,
this resolves the fixme first mentioned in commit 367cd69 (even
though I accidentally removed the fixme comment early in 74430fe).
* src/util/virstoragefile.c (virStorageFileChainLookup): Depend on
new rather than old fields.
* tests/virstoragetest.c (mymain): Adjust test to match fix.
Signed-off-by: Eric Blake <eblake@redhat.com>
The original chain lookup code had to pass in the starting name,
because it was not available in the chain. But now that we have
added fields to the struct, this parameter is redundant.
* src/util/virstoragefile.h (virStorageFileChainLookup): Alter
signature.
* src/util/virstoragefile.c (virStorageFileChainLookup): Adjust
handling of top of chain.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Adjust caller.
* tests/virstoragetest.c (testStorageLookup, mymain): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
The chain lookup function was inconsistent on whether it left
a message in the log when looking up a name that is not found
on the chain (leaving a message for OOM or if name was
relative but not part of the chain), and could litter the log
even when successful (when name was relative but deep in the
chain, use of virFindBackingFile early in the chain would complain
about a file not found). It's easier to make the function
consistently emit a message exactly once on failure, and to let
all callers rely on the clean semantics.
* src/util/virstoragefile.c (virStorageFileChainLookup): Always
report error on failure. Simplify relative lookups.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Avoid
overwriting error.
Signed-off-by: Eric Blake <eblake@redhat.com>
When checking if two filenames point to the same inode (whether
by hardlink or symlink), sometimes one of the names might be
relative. This convenience function makes it easier to check.
* src/util/virfile.h (virFileRelLinkPointsTo): New prototype.
* src/util/virfile.c (virFileRelLinkPointsTo): New function.
* src/libvirt_private.syms (virfile.h): Export it.
* src/xen/xm_internal.c (xenXMDomainGetAutostart): Use it.
Signed-off-by: Eric Blake <eblake@redhat.com>
Drop another redundant field from virStorageFileMetadata.
* src/util/virstoragefile.h (_virStorageFileMetadata): Drop
field.
* src/util/virstoragefile.c
(virStorageFileGetMetadataFromFDInternal)
(virStorageFileGetMetadataFromFD)
(virStorageFileGetMetadataRecurse): Adjust callers.
* tests/virstoragetest.c (_testFileData, testStorageChain)
(mymain): Simplify test.
Signed-off-by: Eric Blake <eblake@redhat.com>
A couple pieces of virStorageFileMetadata are used only while
collecting information about the chain, and don't need to
live permanently in the struct. This patch refactors external
callers to collect the information separately, so that the
next patch can remove the fields.
* src/util/virstoragefile.h (virStorageFileGetMetadataFromBuf):
Alter signature.
* src/util/virstoragefile.c (virStorageFileGetMetadataInternal):
Likewise.
(virStorageFileGetMetadataFromFDInternal): Adjust callers.
* src/storage/storage_backend_fs.c (virStorageBackendProbeTarget):
Likewise.
* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshVol): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Finally starting to prune away some of the old fields that have
been made redundant by the new fields, on my way towards directly
reusing virStorageSource.
* src/util/virstoragefile.h (_virStorageFileMetadata): Drop
field.
* src/util/virstoragefile.c (virStorageFileGetMetadataInternal)
(virStorageFileChainLookup): Adjust callers.
* tests/virstoragetest.c (_testFileData, testStorageChain)
(mymain): Simplify test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Deciding if a user string represents a local file instead of a
network path is an operation worth exposing directly, particularly
since the next patch will be removing a redundant variable that
was caching the information.
* src/util/virstoragefile.h (virStorageIsFile): New declaration.
* src/util/virstoragefile.c (virBackingStoreIsFile): Rename...
(virStorageIsFile): ...export, and allow NULL input.
(virStorageFileGetMetadataInternal)
(virStorageFileGetMetadataRecurse, virStorageFileGetMetadata):
Update callers.
* src/conf/domain_conf.c (virDomainDiskDefForeachPath): Use it.
* src/storage/storage_backend_fs.c (virStorageBackendProbeTarget):
Likewise.
* src/libvirt_private.syms (virstoragefile.h): Export function.
Signed-off-by: Eric Blake <eblake@redhat.com>
So far, my work has been merely preserving the status quo of
backing file analysis. But this patch starts to tread in the
territory of making the backing chain code more powerful - we
will eventually support network storage containing non-raw
formats. Here, we expose metadata information about a network
backing store, even if that information is still hardcoded to
a raw format for now.
* src/util/virstoragefile.c (virStorageFileGetMetadataRecurse):
Also populate struct for non-file backing.
(virStorageFileGetMetadata, virStorageFileGetMetadatainternal):
Recognize non-file top image.
(virFindBackingFile): Add comment.
(virStorageFileChainGetBroken): Adjust comment, ensure output
is set.
* tests/virstoragetest.c (mymain): Update test to reflect it.
Signed-off-by: Eric Blake <eblake@redhat.com>
Most of the APIs in CPU driver do not expect to get NULL for input
parameters. Let's mark them with ATTRIBUTE_NONNULL and also check for
some members of virCPUDef when the APIs expect them have some specific
values.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The iterator is checked for being less than or equal to need_cpus.
The 'n' variable is incremented need_cpus + 1 times.
Simplify the computation of need_cpus and make its value one larger,
to let it be used instead of 'n' and compared without the equal sign
in loop conditions.
Just index the sum_cpu_time array instead of using a helper variable.
Start the loop at start_cpu instead of continuing for all lower values.
