[libvirt] [PATCH] Restore skipping of setting capacity
by John Ferlan
Commit id 'ac9a0963' refactored out the 'withCapacity' for the
virStorageBackendUpdateVolInfo() API. See:
http://www.redhat.com/archives/libvir-list/2014-April/msg00043.html
This resulted in a difference in how 'virsh vol-info --pool <poolName>
<volume>' or 'virsh vol-list vol-list --pool <poolName> --details' outputs
the capacity information for a directory pool with a qcow2 sparse file.
For example, using the following XML
mkdir /home/TestPool
cat testpool.xml
<pool type='dir'>
<name>TestPool</name>
<uuid>6bf80895-10b6-75a6-6059-89fdea2aefb7</uuid>
<source>
</source>
<target>
<path>/home/TestPool</path>
<permissions>
<mode>0755</mode>
<owner>0</owner>
<group>0</group>
</permissions>
</target>
</pool>
virsh pool-create testpool.xml
virsh vol-create-as --pool TestPool temp_vol_1 \
--capacity 1048576 --allocation 1048576 --format qcow2
virsh vol-info --pool TestPool temp_vol_1
Results in listing a Capacity value. Prior to the commit, the value would
be '1.0 MiB' (1048576 bytes). However, after the commit the output would be
(for example) '192.50 KiB', which for my system was the size of the volume
in my file system (eg 'ls -l TestPool/temp_vol_1' results in '197120' bytes
or 192.50 KiB). While perhaps technically correct, it's not necessarily
what the user expected (certainly virt-test didn't expect it).
This patch restores the code to not update the target capacity for this path
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend.c | 22 +++++++++++++++-------
src/storage/storage_backend.h | 5 ++++-
src/storage/storage_backend_disk.c | 2 +-
src/storage/storage_backend_fs.c | 11 +++++++----
src/storage/storage_backend_gluster.c | 2 +-
src/storage/storage_backend_logical.c | 2 +-
src/storage/storage_backend_mpath.c | 2 +-
src/storage/storage_backend_scsi.c | 2 +-
8 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 946196b..afedbf5 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1403,6 +1403,7 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
int
virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target,
+ bool updateCapacity,
bool withBlockVolFormat,
unsigned int openflags)
{
@@ -1413,7 +1414,8 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target,
goto cleanup;
fd = ret;
- if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb)) < 0)
+ if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb,
+ updateCapacity)) < 0)
goto cleanup;
if (withBlockVolFormat) {
@@ -1429,18 +1431,21 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target,
int
virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
+ bool updateCapacity,
bool withBlockVolFormat,
unsigned int openflags)
{
int ret;
if ((ret = virStorageBackendUpdateVolTargetInfo(&vol->target,
+ updateCapacity,
withBlockVolFormat,
openflags)) < 0)
return ret;
if (vol->backingStore.path &&
(ret = virStorageBackendUpdateVolTargetInfo(&vol->backingStore,
+ updateCapacity,
withBlockVolFormat,
VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0)
return ret;
@@ -1453,15 +1458,15 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
* @target: target definition ptr of volume to update
* @fd: fd of storage volume to update, via virStorageBackendOpenVol*, or -1
* @sb: details about file (must match @fd, if that is provided)
- * @allocation: If not NULL, updated allocation information will be stored
- * @capacity: If not NULL, updated capacity info will be stored
+ * @updateCapacity: If true, updated capacity info will be stored
*
* Returns 0 for success, -1 on a legitimate error condition.
*/
int
virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target,
int fd,
- struct stat *sb)
+ struct stat *sb,
+ bool updateCapacity)
{
#if WITH_SELINUX
security_context_t filecon = NULL;
@@ -1477,10 +1482,12 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target,
/* Regular files may be sparse, so logical size (capacity) is not same
* as actual allocation above
*/
- target->capacity = sb->st_size;
+ if (updateCapacity)
+ target->capacity = sb->st_size;
} else if (S_ISDIR(sb->st_mode)) {
target->allocation = 0;
- target->capacity = 0;
+ if (updateCapacity)
+ target->capacity = 0;
} else if (fd >= 0) {
off_t end;
/* XXX this is POSIX compliant, but doesn't work for CHAR files,
@@ -1496,7 +1503,8 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target,
return -1;
}
target->allocation = end;
- target->capacity = end;
+ if (updateCapacity)
+ target->capacity = end;
}
if (!target->perms && VIR_ALLOC(target->perms) < 0)
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 5997077..456b9d7 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -137,14 +137,17 @@ int virStorageBackendVolOpen(const char *path, struct stat *sb,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
+ bool updateCapacity,
bool withBlockVolFormat,
unsigned int openflags);
int virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target,
+ bool updateCapacity,
bool withBlockVolFormat,
unsigned int openflags);
int virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target,
int fd,
- struct stat *sb);
+ struct stat *sb,
+ bool updateCapacity);
char *virStorageBackendStablePath(virStoragePoolObjPtr pool,
const char *devpath,
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 9cebcca..13336fc 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -113,7 +113,7 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
}
/* Refresh allocation/capacity/perms */
- if (virStorageBackendUpdateVolInfo(vol, false,
+ if (virStorageBackendUpdateVolInfo(vol, true, false,
VIR_STORAGE_VOL_OPEN_DEFAULT) < 0)
return -1;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 3694c26..5a2add8 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -84,7 +84,8 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
goto error; /* Take care to propagate ret, it is not always -1 */
fd = ret;
- if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb)) < 0) {
+ if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd,
+ &sb, true)) < 0) {
goto error;
}
@@ -913,7 +914,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED,
vol->backingStore.format = backingStoreFormat;
ignore_value(virStorageBackendUpdateVolTargetInfo(
- &vol->backingStore, false,
+ &vol->backingStore, true, false,
VIR_STORAGE_VOL_OPEN_DEFAULT));
/* If this failed, the backing file is currently unavailable,
* the capacity, allocation, owner, group and mode are unknown.
@@ -1190,8 +1191,10 @@ virStorageBackendFileSystemVolRefresh(virConnectPtr conn,
{
int ret;
- /* Refresh allocation / permissions info in case its changed */
- ret = virStorageBackendUpdateVolInfo(vol, false,
+ /* Refresh allocation / permissions info in case its changed
+ * don't update the capacity value for this pass
+ */
+ ret = virStorageBackendUpdateVolInfo(vol, false, false,
VIR_STORAGE_VOL_FS_OPEN_FLAGS);
if (ret < 0)
return ret;
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
index e0a25df..28db909 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -267,7 +267,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
if (VIR_ALLOC(vol) < 0)
goto cleanup;
- if (virStorageBackendUpdateVolTargetInfoFD(&vol->target, -1, st) < 0)
+ if (virStorageBackendUpdateVolTargetInfoFD(&vol->target, -1, st, true) < 0)
goto cleanup;
if (virStorageBackendGlusterSetMetadata(state, vol, name) < 0)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index ed3a012..a597e67 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -149,7 +149,7 @@ virStorageBackendLogicalMakeVol(char **const groups,
if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0)
goto cleanup;
- if (virStorageBackendUpdateVolInfo(vol, false,
+ if (virStorageBackendUpdateVolInfo(vol, true, false,
VIR_STORAGE_VOL_OPEN_DEFAULT) < 0)
goto cleanup;
diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c
index f0ed189..8c3b0df 100644
--- a/src/storage/storage_backend_mpath.c
+++ b/src/storage/storage_backend_mpath.c
@@ -60,7 +60,7 @@ virStorageBackendMpathNewVol(virStoragePoolObjPtr pool,
if (virAsprintf(&vol->target.path, "/dev/%s", dev) < 0)
goto cleanup;
- if (virStorageBackendUpdateVolInfo(vol, true,
+ if (virStorageBackendUpdateVolInfo(vol, true, true,
VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) {
goto cleanup;
}
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index c448d7f..71bcf56 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -199,7 +199,7 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
goto free_vol;
}
- if (virStorageBackendUpdateVolInfo(vol, true,
+ if (virStorageBackendUpdateVolInfo(vol, true, true,
VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) {
retval = -1;
goto free_vol;
--
1.9.0
10 years, 6 months
[libvirt] Libvirt API
by Vikas Kokare
We are seeing issues(data is unavailable) with usage of the following
Libvirt APIs, on RHEL5, while they are working Ok with RHEL 6, SLES10/11.
CPU dumpxml from Domain.getXMLDesc() returns no xml nodes for CPU model and
topology and its sub-elements such as sockets, cores, threads.
Domain.memoryStats(int) : added since 0.5.0
Please guide.
10 years, 6 months
[libvirt] Test failures with newest gnutls
by Martin Kletzander
Hi everyone,
after upgrade to gnutls-3.3.0, I discovered (commandtest fails) that
any code linked with -lgnutls will have not 3, but 5 open file
descriptors upon the entry into main(). I asked on gnutls-help [1] if
they know they are leaking file descriptors. The response was, that
this is intended with the explanation being that these FDs (pointing
to /dev/urandom) are kept open for backward compatibility with
programs that may chroot into environment without /dev/urandom as the
previous version didn't require to have access to /dev/urandom when
calling gnutls code.
Does that seem like our bug that we're relying on fixed number of open
file descriptors? Or that we're linking to gnutls when we don't need
it in commandhelper? Or should this be fixed somewhere else?
Have a nice day,
Martin
[1] http://lists.gnutls.org/pipermail/gnutls-help/2014-April/003415.html
10 years, 6 months
[libvirt] [PATCH v6 0/6] Expose FSFreeze/FSThaw within the guest as API
by Tomoki Sekiyama
Hello,
This is patchset v6 to add FSFreeze/FSThaw API for custom disk snapshotting.
Changes since v5:
* replace disks and ndisks parameter with mountpoints and nmountpoints
to specify filesystem to be frozen/thawed, so that we can specify
non-disk-backed filesystems
* add PATCH 6 to support 'mountpoints' argument for
'guest-fsfreeze-freeze' command of qemu-guest-agent.
(This requires a patch for qemu-guest-agent, which isn't merged yet:
https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg04434.html )
* drop nested fsfreeze request feature
* change API version to 1.2.5
(v5: https://www.redhat.com/archives/libvir-list/2014-April/msg00140.html )
=== Description ===
Currently FSFreeze and FSThaw are supported by qemu guest agent and they
are used internally in snapshot-create command with --quiesce option.
However, when users want to utilize the native snapshot feature of storage
devices (such as LVM over iSCSI, enterprise storage appliances, etc.),
they need to issue fsfreeze command separately from libvirt-driven snapshots.
(OpenStack cinder provides these storages' snapshot feature, but it cannot
quiesce the guest filesystems automatically for now.)
Although virDomainQemuGuestAgent() API could be used for this purpose, it
is only for debugging and is not supported officially.
This patchset adds virDomainFSFreeze()/virDomainFSThaw() APIs and virsh
domfsfreeze/domfsthaw commands to enable the users to freeze and thaw
domain's filesystems cleanly.
<updated>
The APIs take disks and ndisks parameters, which is a list of mountpoints
of filesystems to be frozen/thawed. If the option is not provided, or
guest agent doesn't support the 'mountpoints' argument, every mounted
filesystem is frozen/thawed.
</updated>
The APIs have flags option currently unsupported for future extension.
---
Tomoki Sekiyama (6):
Introduce virDomainFSFreeze() and virDomainFSThaw() public API
remote: Implement virDomainFSFreeze and virDomainFSThaw
qemu: track quiesced status in qemuDomainSnapshotFSFreeze
qemu: Implement virDomainFSFreeze and virDomainFSThaw
virsh: Expose new virDomainFSFreeze and virDomainFSThaw API
qemu: Support mountpoints option of guest-fsfreeze-freeze
include/libvirt/libvirt.h.in | 10 +++
src/access/viraccessperm.c | 2 -
src/access/viraccessperm.h | 6 ++
src/driver.h | 14 ++++
src/libvirt.c | 93 ++++++++++++++++++++++++
src/libvirt_public.syms | 6 ++
src/qemu/qemu_agent.c | 47 +++++++++++-
src/qemu/qemu_agent.h | 3 +
src/qemu/qemu_domain.c | 5 +
src/qemu/qemu_domain.h | 2 +
src/qemu/qemu_driver.c | 160 ++++++++++++++++++++++++++++++++++++++----
src/remote/remote_driver.c | 2 +
src/remote/remote_protocol.x | 38 +++++++++-
src/remote_protocol-structs | 18 +++++
src/rpc/gendispatch.pl | 2 +
tests/qemuagenttest.c | 8 +-
tools/virsh-domain.c | 130 ++++++++++++++++++++++++++++++++++
tools/virsh.pod | 23 ++++++
18 files changed, 544 insertions(+), 25 deletions(-)
10 years, 6 months
[libvirt] [PATCH v7 0/4] Handling of undefine and re define snapshots with VirtualBox 4.2 or higher
by Yohan BELLEGUIC
Hello,
This is a new series of patches in order to support undefining and redefining
snapshots with VirtualBox 4.2 or higher.
These patches are based on Manuel Vives' patches, taking into account Daniel
P. Berrange's remarks.
The VirtualBox API provides only high level operations to manipulate snapshots,
so it not possible to support flags like VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE and
VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY with only API calls.
Following an IRC talk with Eric Blake, the decision was made to emulate these
behaviours by manipulating directly the .vbox XML files.
The first patch adds extra details in the snapshot XML returned by libvirt. We
will need those details in order to redefine the snapshots.
The second patch adds a new API to manipulate the VirtualBox XML file. It
provides several structs describing the VirtualBox XML file nodes and
functions which can manipulate these structs.
The third patch adds support of the VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE
and VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT flags in virDomainSnapshotCreateXML.
The idea is to unregister the machine, add the snapshot in the Virtualbox XML
file and re-register the machine.
However, VirtualBox forbids a machine to have snapshots but no current
snapshot. So, if the real current snapshot has not been redefined yet, we
create fake disks, allowing us to have an intermediate state in order to not
corrupt the snapshot's read-write disks. These fake disks will be deleted during the
next redefine.
The fourth and last patch adds support of the
VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY flag in virDomainSnapshotDelete.
As in the third patch, we also create fake disks to not corrupt the snapshot's read-write disks.
The patches were only tested with VirtualBox 4.3.10 and VirtualBox 4.2.24.
Regards
Yohan BELLEGUIC
v7:
* Add vbox_snapshot_conf.{h,c} files to (de)serialize VirtualBox XML files
* Update the code to use the API exposed by vbox_snapshot_conf.h
* Handle the fact that VirtualBox forbids a machine to have snapshots but no
current snapshot
v6:
* Rebased because of a massive change in vbox_tmpl.c due to changes in
the handling of different versions of VirtualBox
v5:
* The patches are modified according to a first review by Laine Stump:
* renamed virSearchUuid to virSearchRegex and moved it from
viruuid.{c,h} to virstring.{c,h}.
* Various fixes.
v4:
* The code is compliant with Virtualbox 4.3 API
* Some minor modifications in order to satisfy "make syntax-check"
v3:
* Use of STREQ_NULLABLE instead of STREQ in one case
* Fix the method for finding uuids according to Ján Tomko review
v2:
* Fix a licence problem with the method for string replacement
Manuel VIVES (1):
vbox_tmpl.c: Better XML description for snapshots
Yohan BELLEGUIC (3):
Add vbox_snapshot_conf struct
vbox_tmpl.c: Patch for redefining snapshots
vbox_tmpl.c: Add function for undefining snapshot
po/POTFILES.in | 1 +
src/Makefile.am | 1 +
src/vbox/vbox_snapshot_conf.c | 1383 +++++++++++++++++++++++++
src/vbox/vbox_snapshot_conf.h | 100 ++
src/vbox/vbox_tmpl.c | 2225 +++++++++++++++++++++++++++++++++++++----
5 files changed, 3538 insertions(+), 172 deletions(-)
create mode 100644 src/vbox/vbox_snapshot_conf.c
create mode 100644 src/vbox/vbox_snapshot_conf.h
--
1.7.10.4
10 years, 6 months
[libvirt] [PATCH] conf: handle 'vda[-1]' uniformly
by Eric Blake
Commit f22b7899 stumbled across a difference between 32-bit and
64-bit platforms; on 64-bit, parsing "-1" as a long produces
0xffffffffffffffff, which does not fit in unsigned int; but on
32-bit, it parses as 0xffffffff, which DOES fit. Another patch
will tweak virStrToLong_ui to behave the same across platforms,
but regardless of which of the two choices it makes, the chain
lookup code wants to reject negative numbers rather than treating
it as large integers.
* src/util/virstoragefile.c (virStorageFileParseChainIndex):
Reject negative sign.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
Therefore, although this qualifies as a build-breaker fix on
32-bit, I haven't pushed it yet: I'm still working on my patch
to virstring.c for uniform behavior; and that turned up the
fact that virstringtest.c doesn't have coverage of integer
parsing, so of course, I have to expand that test too...
src/util/virstoragefile.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index dcce1ef..5d933a4 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1525,7 +1525,9 @@ virStorageFileParseChainIndex(const char *diskTarget,
if (virStringListLength(strings) != 2)
goto cleanup;
- if (virStrToLong_ui(strings[1], &suffix, 10, &idx) < 0 ||
+ /* Rule out spaces or negative sign */
+ if (!c_isdigit(*strings[1]) ||
+ virStrToLong_ui(strings[1], &suffix, 10, &idx) < 0 ||
STRNEQ(suffix, "]"))
goto cleanup;
--
1.9.0
10 years, 6 months
[libvirt] [PATCH for 1.2.4] storage: Clear all data allocated about backing store before reparsing
by Peter Krempa
To avoid memory leak of the "backingStoreRaw" field when reparsing
backing chains a new function is being introduced by this patch that
shall be used to clear backing store information.
The memory leak was introduced in commit 8823272d41a259c1246c05d.
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_domain.c | 8 +++-----
src/qemu/qemu_driver.c | 3 +--
src/util/virstoragefile.c | 30 ++++++++++++++++++++++++------
src/util/virstoragefile.h | 1 +
5 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 55b016d..4cfaefc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1865,6 +1865,7 @@ virStorageNetHostTransportTypeToString;
virStorageNetProtocolTypeToString;
virStorageSourceAuthClear;
virStorageSourceClear;
+virStorageSourceClearBackingStore;
virStorageSourceFree;
virStorageSourceGetActualType;
virStorageSourcePoolDefFree;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8fa58f3..a3c1b1c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2421,12 +2421,10 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
goto cleanup;
if (disk->src.backingStore) {
- if (force) {
- virStorageSourceFree(disk->src.backingStore);
- disk->src.backingStore = NULL;
- } else {
+ if (force)
+ virStorageSourceClearBackingStore(&disk->src);
+ else
goto cleanup;
- }
}
qemuDomainGetImageIds(cfg, vm, disk, &uid, &gid);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 75cf8cb..bf19c6e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12734,8 +12734,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
* recompute it. Better would be storing the chain ourselves rather than
* reprobing, but this requires modifying domain_conf and our XML to fully
* track the chain across libvirtd restarts. */
- virStorageSourceFree(disk->src.backingStore);
- disk->src.backingStore = NULL;
+ virStorageSourceClearBackingStore(&disk->src);
if (virStorageFileInit(&snap->src) < 0)
goto cleanup;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index dcce1ef..5c43665 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1726,6 +1726,29 @@ virStorageSourceGetActualType(virStorageSourcePtr def)
}
+/**
+ * virStorageSourceClearBackingStore:
+ *
+ * @src: disk source to clear
+ *
+ * Clears information about backing store of the current storage file.
+ */
+void
+virStorageSourceClearBackingStore(virStorageSourcePtr def)
+{
+ if (!def)
+ return;
+
+ VIR_FREE(def->relPath);
+ VIR_FREE(def->relDir);
+ VIR_FREE(def->backingStoreRaw);
+
+ /* recursively free backing chain */
+ virStorageSourceFree(def->backingStore);
+ def->backingStore = NULL;
+}
+
+
void
virStorageSourceClear(virStorageSourcePtr def)
{
@@ -1755,12 +1778,7 @@ virStorageSourceClear(virStorageSourcePtr def)
virStorageNetHostDefFree(def->nhosts, def->hosts);
virStorageSourceAuthClear(def);
- VIR_FREE(def->relPath);
- VIR_FREE(def->relDir);
- VIR_FREE(def->backingStoreRaw);
-
- /* recursively free backing chain */
- virStorageSourceFree(def->backingStore);
+ virStorageSourceClearBackingStore(def);
}
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 148776e..9e6cdba 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -316,5 +316,6 @@ void virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def);
void virStorageSourceClear(virStorageSourcePtr def);
int virStorageSourceGetActualType(virStorageSourcePtr def);
void virStorageSourceFree(virStorageSourcePtr def);
+void virStorageSourceClearBackingStore(virStorageSourcePtr def);
#endif /* __VIR_STORAGE_FILE_H__ */
--
1.9.1
10 years, 6 months
[libvirt] [PATCH v2] nwfilter: Tear down temp. filters when tearing all filters
by Stefan Berger
From: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
Refactor the ebiptablesTearNewRules function so that the teardown of temporary
filters can also be called by the ebiptablesAllTeardown function.
This fixes a problem that leaves temporary filters behind when a VM shuts down
while its filters are modified.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
v1->v2:
- test cases adjusted to expect more commands
---
src/nwfilter/nwfilter_ebiptables_driver.c | 24 ++++++++----
tests/nwfilterebiptablestest.c | 64 +++++++++++++++++--------------
2 files changed, 52 insertions(+), 36 deletions(-)
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
index 4cbdc0e..5cb0b74 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -3536,14 +3536,9 @@ ebiptablesApplyNewRules(const char *ifname,
}
-static int
-ebiptablesTearNewRules(const char *ifname)
+static void
+ebiptablesTearNewRulesFW(virFirewallPtr fw, const char *ifname)
{
- virFirewallPtr fw = virFirewallNew();
- int ret = -1;
-
- virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS);
-
iptablesUnlinkTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname);
iptablesRemoveTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname);
@@ -3555,13 +3550,24 @@ ebiptablesTearNewRules(const char *ifname)
ebtablesRemoveTmpSubChainsFW(fw, ifname);
ebtablesRemoveTmpRootChainFW(fw, true, ifname);
ebtablesRemoveTmpRootChainFW(fw, false, ifname);
+}
+
+
+static int
+ebiptablesTearNewRules(const char *ifname)
+{
+ virFirewallPtr fw = virFirewallNew();
+ int ret = -1;
+
+ virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS);
+
+ ebiptablesTearNewRulesFW(fw, ifname);
ret = virFirewallApply(fw);
virFirewallFree(fw);
return ret;
}
-
static int
ebiptablesTearOldRules(const char *ifname)
{
@@ -3608,6 +3614,8 @@ ebiptablesAllTeardown(const char *ifname)
virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS);
+ ebiptablesTearNewRulesFW(fw, ifname);
+
iptablesUnlinkRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname);
iptablesClearVirtInPostFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname);
iptablesRemoveRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname);
diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c
index df939d5..ff9a9b7 100644
--- a/tests/nwfilterebiptablestest.c
+++ b/tests/nwfilterebiptablestest.c
@@ -33,11 +33,43 @@
#define VIR_FROM_THIS VIR_FROM_NONE
+
+#define VIR_NWFILTER_NEW_RULES_TEARDOWN \
+ "iptables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FP-vnet0\n" \
+ "iptables -D libvirt-out -m physdev --physdev-out vnet0 -g FP-vnet0\n" \
+ "iptables -D libvirt-in -m physdev --physdev-in vnet0 -g FJ-vnet0\n" \
+ "iptables -D libvirt-host-in -m physdev --physdev-in vnet0 -g HJ-vnet0\n" \
+ "iptables -F FP-vnet0\n" \
+ "iptables -X FP-vnet0\n" \
+ "iptables -F FJ-vnet0\n" \
+ "iptables -X FJ-vnet0\n" \
+ "iptables -F HJ-vnet0\n" \
+ "iptables -X HJ-vnet0\n" \
+ "ip6tables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FP-vnet0\n" \
+ "ip6tables -D libvirt-out -m physdev --physdev-out vnet0 -g FP-vnet0\n" \
+ "ip6tables -D libvirt-in -m physdev --physdev-in vnet0 -g FJ-vnet0\n" \
+ "ip6tables -D libvirt-host-in -m physdev --physdev-in vnet0 -g HJ-vnet0\n" \
+ "ip6tables -F FP-vnet0\n" \
+ "ip6tables -X FP-vnet0\n" \
+ "ip6tables -F FJ-vnet0\n" \
+ "ip6tables -X FJ-vnet0\n" \
+ "ip6tables -F HJ-vnet0\n" \
+ "ip6tables -X HJ-vnet0\n" \
+ "ebtables -t nat -D PREROUTING -i vnet0 -j libvirt-J-vnet0\n" \
+ "ebtables -t nat -D POSTROUTING -o vnet0 -j libvirt-P-vnet0\n" \
+ "ebtables -t nat -L libvirt-J-vnet0\n" \
+ "ebtables -t nat -L libvirt-P-vnet0\n" \
+ "ebtables -t nat -F libvirt-J-vnet0\n" \
+ "ebtables -t nat -X libvirt-J-vnet0\n" \
+ "ebtables -t nat -F libvirt-P-vnet0\n" \
+ "ebtables -t nat -X libvirt-P-vnet0\n"
+
static int
testNWFilterEBIPTablesAllTeardown(const void *opaque ATTRIBUTE_UNUSED)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
const char *expected =
+ VIR_NWFILTER_NEW_RULES_TEARDOWN
"iptables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FO-vnet0\n"
"iptables -D libvirt-out -m physdev --physdev-out vnet0 -g FO-vnet0\n"
"iptables -D libvirt-in -m physdev --physdev-in vnet0 -g FI-vnet0\n"
@@ -221,34 +253,7 @@ testNWFilterEBIPTablesTearNewRules(const void *opaque ATTRIBUTE_UNUSED)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
const char *expected =
- "iptables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FP-vnet0\n"
- "iptables -D libvirt-out -m physdev --physdev-out vnet0 -g FP-vnet0\n"
- "iptables -D libvirt-in -m physdev --physdev-in vnet0 -g FJ-vnet0\n"
- "iptables -D libvirt-host-in -m physdev --physdev-in vnet0 -g HJ-vnet0\n"
- "iptables -F FP-vnet0\n"
- "iptables -X FP-vnet0\n"
- "iptables -F FJ-vnet0\n"
- "iptables -X FJ-vnet0\n"
- "iptables -F HJ-vnet0\n"
- "iptables -X HJ-vnet0\n"
- "ip6tables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FP-vnet0\n"
- "ip6tables -D libvirt-out -m physdev --physdev-out vnet0 -g FP-vnet0\n"
- "ip6tables -D libvirt-in -m physdev --physdev-in vnet0 -g FJ-vnet0\n"
- "ip6tables -D libvirt-host-in -m physdev --physdev-in vnet0 -g HJ-vnet0\n"
- "ip6tables -F FP-vnet0\n"
- "ip6tables -X FP-vnet0\n"
- "ip6tables -F FJ-vnet0\n"
- "ip6tables -X FJ-vnet0\n"
- "ip6tables -F HJ-vnet0\n"
- "ip6tables -X HJ-vnet0\n"
- "ebtables -t nat -D PREROUTING -i vnet0 -j libvirt-J-vnet0\n"
- "ebtables -t nat -D POSTROUTING -o vnet0 -j libvirt-P-vnet0\n"
- "ebtables -t nat -L libvirt-J-vnet0\n"
- "ebtables -t nat -L libvirt-P-vnet0\n"
- "ebtables -t nat -F libvirt-J-vnet0\n"
- "ebtables -t nat -X libvirt-J-vnet0\n"
- "ebtables -t nat -F libvirt-P-vnet0\n"
- "ebtables -t nat -X libvirt-P-vnet0\n";
+ VIR_NWFILTER_NEW_RULES_TEARDOWN;
char *actual = NULL;
int ret = -1;
@@ -282,6 +287,7 @@ testNWFilterEBIPTablesApplyBasicRules(const void *opaque ATTRIBUTE_UNUSED)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
const char *expected =
+ VIR_NWFILTER_NEW_RULES_TEARDOWN
"iptables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FO-vnet0\n"
"iptables -D libvirt-out -m physdev --physdev-out vnet0 -g FO-vnet0\n"
"iptables -D libvirt-in -m physdev --physdev-in vnet0 -g FI-vnet0\n"
@@ -353,6 +359,7 @@ testNWFilterEBIPTablesApplyDHCPOnlyRules(const void *opaque ATTRIBUTE_UNUSED)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
const char *expected =
+ VIR_NWFILTER_NEW_RULES_TEARDOWN
"iptables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FO-vnet0\n"
"iptables -D libvirt-out -m physdev --physdev-out vnet0 -g FO-vnet0\n"
"iptables -D libvirt-in -m physdev --physdev-in vnet0 -g FI-vnet0\n"
@@ -443,6 +450,7 @@ testNWFilterEBIPTablesApplyDropAllRules(const void *opaque ATTRIBUTE_UNUSED)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
const char *expected =
+ VIR_NWFILTER_NEW_RULES_TEARDOWN
"iptables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FO-vnet0\n"
"iptables -D libvirt-out -m physdev --physdev-out vnet0 -g FO-vnet0\n"
"iptables -D libvirt-in -m physdev --physdev-in vnet0 -g FI-vnet0\n"
--
1.8.1.4
10 years, 6 months
[libvirt] [PATCHv2] Device{Attach, Detach}: Document S4 limitations
by Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=808463
Well, libvirt doesn't distinguish between domain poweroff and
hibernation (S4). It's hard to differentiate these two on a real
machine anyway. As a result, any device that is hot(un-)plugged is
lost (appears again) when domain is started again as from our POV
it is a fresh cold boot. Instead of doing anything wise here, we
should just document this as known limitation.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/src/libvirt.c b/src/libvirt.c
index b6c99c5..79071db 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -10319,6 +10319,10 @@ virNodeGetSecurityModel(virConnectPtr conn, virSecurityModelPtr secmodel)
* in an existing CDROM/Floppy device, however, applications are
* recommended to use the virDomainUpdateDeviceFlag method instead.
*
+ * Be aware that hotplug changes might not persist across a domain going
+ * into S4 state (also known as hibernation) unless you also modify the
+ * persistent domain definition.
+ *
* Returns 0 in case of success, -1 in case of failure.
*/
int
@@ -10374,6 +10378,10 @@ virDomainAttachDevice(virDomainPtr domain, const char *xml)
* in an existing CDROM/Floppy device, however, applications are
* recommended to use the virDomainUpdateDeviceFlag method instead.
*
+ * Be aware that hotplug changes might not persist across a domain going
+ * into S4 state (also known as hibernation) unless you also modify the
+ * persistent domain definition.
+ *
* Returns 0 in case of success, -1 in case of failure.
*/
int
@@ -10416,6 +10424,10 @@ virDomainAttachDeviceFlags(virDomainPtr domain,
* Destroy a virtual device attachment to backend. This function,
* having hot-unplug semantics, is only allowed on an active domain.
*
+ * Be aware that hotplug changes might not persist across a domain going
+ * into S4 state (also known as hibernation) unless you also modify the
+ * persistent domain definition.
+ *
* Returns 0 in case of success, -1 in case of failure.
*/
int
@@ -10487,6 +10499,10 @@ virDomainDetachDevice(virDomainPtr domain, const char *xml)
* a synchronous removal. In other words, this API may wait a bit for the
* removal to complete in case it was not synchronous.
*
+ * Be aware that hotplug changes might not persist across a domain going
+ * into S4 state (also known as hibernation) unless you also modify the
+ * persistent domain definition.
+ *
* Returns 0 in case of success, -1 in case of failure.
*/
int
--
1.9.0
10 years, 6 months
[libvirt] [PATCH] Device{Attach,Detach}: Document S4 limitations
by Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=808463
Well, libvirt doesn't distinguish between domain poweroff and
hibernation (S4). It's hard to differentiate these two on a real
machine anyway. As a result, any device that is hot(un-)plugged is
lost (appears again) when domain is started again as from our POV
it is a fresh cold boot. Instead of doing anything wise here, we
should just document this as known limitation.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/libvirt.c b/src/libvirt.c
index b6c99c5..20297eb 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -10319,6 +10319,9 @@ virNodeGetSecurityModel(virConnectPtr conn, virSecurityModelPtr secmodel)
* in an existing CDROM/Floppy device, however, applications are
* recommended to use the virDomainUpdateDeviceFlag method instead.
*
+ * Be aware that a device that is hotplugged may disappear if
+ * domain resumes from the S4 state (also known as hibernation).
+ *
* Returns 0 in case of success, -1 in case of failure.
*/
int
@@ -10374,6 +10377,9 @@ virDomainAttachDevice(virDomainPtr domain, const char *xml)
* in an existing CDROM/Floppy device, however, applications are
* recommended to use the virDomainUpdateDeviceFlag method instead.
*
+ * Be aware that a device that is hotplugged may disappear if
+ * domain resumes from the S4 state (also known as hibernation).
+ *
* Returns 0 in case of success, -1 in case of failure.
*/
int
@@ -10416,6 +10422,9 @@ virDomainAttachDeviceFlags(virDomainPtr domain,
* Destroy a virtual device attachment to backend. This function,
* having hot-unplug semantics, is only allowed on an active domain.
*
+ * Be aware that a device that was unplugged may appear again
+ * if domain resumes from the S4 state (also known as hibernation).
+ *
* Returns 0 in case of success, -1 in case of failure.
*/
int
@@ -10487,6 +10496,9 @@ virDomainDetachDevice(virDomainPtr domain, const char *xml)
* a synchronous removal. In other words, this API may wait a bit for the
* removal to complete in case it was not synchronous.
*
+ * Be aware that a device that was unplugged may appear again
+ * if domain resumes from the S4 state (also known as hibernation).
+ *
* Returns 0 in case of success, -1 in case of failure.
*/
int
--
1.9.0
10 years, 6 months
