[libvirt] [PATCH v2] Make logical pools independent on target path
by Martin Kletzander
When using logical pools, we had to trust the target->path provided.
This parameter, however, can be completely ommited and we can get the
correct path using 'lvdisplay' command. In order not to omit the
target.path completely, we rather default it to '/dev/<source.name>'
which is used to check the pool anyway.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=952973
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
Notes:
v2:
- don't drop target.path, but fix it instead to '/dev/<source.name>'
Thanks to the change from v1, we can now safely drop all the hunks
from logical backend and even the dependency on lvdisplay command.
There might be some systems where the path is different and the part
of this patch using lvdisplay command would make most of them work.
However, checkPool() still depends on '/dev/<source.name>' and I
haven't found any other way how to fix that, so feel free to ACK just
the {conf,docs,tests}/ part in case you think that we shouldn't try to
support anything else than '/dev/<source.name>'.
configure.ac | 4 +
docs/schemas/storagepool.rng | 13 ++-
src/conf/storage_conf.c | 19 +++--
src/storage/storage_backend_logical.c | 95 +++++++++++++---------
src/storage/storage_driver.c | 2 +-
tests/storagepoolxml2xmlin/pool-logical-create.xml | 2 +-
tests/storagepoolxml2xmlin/pool-logical-nopath.xml | 18 ++++
.../storagepoolxml2xmlout/pool-logical-nopath.xml | 19 +++++
tests/storagepoolxml2xmltest.c | 1 +
9 files changed, 125 insertions(+), 48 deletions(-)
create mode 100644 tests/storagepoolxml2xmlin/pool-logical-nopath.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-logical-nopath.xml
diff --git a/configure.ac b/configure.ac
index ef246a6..221b6cb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1601,6 +1601,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then
AC_PATH_PROG([PVS], [pvs], [], [$PATH:/sbin:/usr/sbin])
AC_PATH_PROG([VGS], [vgs], [], [$PATH:/sbin:/usr/sbin])
AC_PATH_PROG([LVS], [lvs], [], [$PATH:/sbin:/usr/sbin])
+ AC_PATH_PROG([LVDISPLAY], [lvdisplay], [], [$PATH:/sbin:/usr/sbin])
if test "$with_storage_lvm" = "yes" ; then
if test -z "$PVCREATE" ; then AC_MSG_ERROR([We need pvcreate for LVM storage driver]) ; fi
@@ -1615,6 +1616,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then
if test -z "$PVS" ; then AC_MSG_ERROR([We need pvs for LVM storage driver]) ; fi
if test -z "$VGS" ; then AC_MSG_ERROR([We need vgs for LVM storage driver]) ; fi
if test -z "$LVS" ; then AC_MSG_ERROR([We need lvs for LVM storage driver]) ; fi
+ if test -z "$LVDISPLAY" ; then AC_MSG_ERROR([We need lvdisplay for LVM storage driver]) ; fi
else
if test -z "$PVCREATE" ; then with_storage_lvm=no ; fi
if test -z "$VGCREATE" ; then with_storage_lvm=no ; fi
@@ -1628,6 +1630,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then
if test -z "$PVS" ; then with_storage_lvm=no ; fi
if test -z "$VGS" ; then with_storage_lvm=no ; fi
if test -z "$LVS" ; then with_storage_lvm=no ; fi
+ if test -z "$LVDISPLAY" ; then with_storage_lvm=no ; fi
if test "$with_storage_lvm" = "check" ; then with_storage_lvm=yes ; fi
fi
@@ -1646,6 +1649,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then
AC_DEFINE_UNQUOTED([PVS],["$PVS"],[Location of pvs program])
AC_DEFINE_UNQUOTED([VGS],["$VGS"],[Location of vgs program])
AC_DEFINE_UNQUOTED([LVS],["$LVS"],[Location of lvs program])
+ AC_DEFINE_UNQUOTED([LVDISPLAY],["$LVDISPLAY"],[Location of lvdisplay program])
fi
fi
AM_CONDITIONAL([WITH_STORAGE_LVM], [test "$with_storage_lvm" = "yes"])
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 3c2158a..1b3f4bc 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -62,7 +62,7 @@
<ref name='commonmetadata'/>
<ref name='sizing'/>
<ref name='sourcelogical'/>
- <ref name='target'/>
+ <ref name='targetlogical'/>
</define>
<define name='pooldisk'>
@@ -207,6 +207,17 @@
</element>
</define>
+ <define name='targetlogical'>
+ <element name='target'>
+ <optional>
+ <element name='path'>
+ <ref name='absFilePath'/>
+ </element>
+ </optional>
+ <ref name='permissions'/>
+ </element>
+ </define>
+
<define name='sourceinfohost'>
<oneOrMore>
<element name='host'>
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 288e265..7ffe58c 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1,7 +1,7 @@
/*
* storage_conf.c: config handling for storage driver
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -959,15 +959,22 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
/* When we are working with a virtual disk we can skip the target
* path and permissions */
if (!(options->flags & VIR_STORAGE_POOL_SOURCE_NETWORK)) {
- if (!(target_path = virXPathString("string(./target/path)", ctxt))) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing storage pool target path"));
- goto error;
+ if (ret->type == VIR_STORAGE_POOL_LOGICAL) {
+ if (virAsprintf(&target_path, "/dev/%s", ret->source.name) < 0) {
+ virReportOOMError();
+ goto error;
+ }
+ } else {
+ target_path = virXPathString("string(./target/path)", ctxt);
+ if (!target_path) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing storage pool target path"));
+ goto error;
+ }
}
ret->target.path = virFileSanitizePath(target_path);
if (!ret->target.path)
goto error;
-
if (virStorageDefParsePerms(ctxt, &ret->target.perms,
"./target/permissions",
DEFAULT_POOL_PERM_MODE) < 0)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index 944aa0e..471f8d9 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -45,6 +45,37 @@
#define PV_BLANK_SECTOR_SIZE 512
+static char *
+virStorageBackendGetVolPath(const char *poolname, const char *volname)
+{
+ char *start = NULL;
+ char *lvpath = NULL;
+ char *output = NULL;
+
+ virCommandPtr cmd = virCommandNewArgList(LVDISPLAY,
+ "--columns",
+ "--options", "lv_path",
+ "--noheadings",
+ "--unbuffered",
+ NULL);
+
+ virCommandAddArgFormat(cmd, "%s/%s", poolname, volname);
+ virCommandSetOutputBuffer(cmd, &output);
+
+ if (virCommandRun(cmd, NULL) < 0 ||
+ !(start = strchr(output, '/'))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot get LV path"));
+ goto cleanup;
+ }
+
+ ignore_value(VIR_STRDUP(lvpath, start));
+
+ cleanup:
+ VIR_FREE(output);
+ virCommandFree(cmd);
+ return lvpath;
+}
+
static int
virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
int on)
@@ -116,11 +147,8 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
}
if (vol->target.path == NULL) {
- if (virAsprintf(&vol->target.path, "%s/%s",
- pool->def->target.path, vol->name) < 0) {
- virReportOOMError();
+ if (VIR_STRDUP(vol->target.path, groups[10]) < 0)
goto cleanup;
- }
}
/* Skips the backingStore of lv created with "--virtualsize",
@@ -130,12 +158,11 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
* (lvs outputs "[$lvname_vorigin] for field "origin" if the
* lv is created with "--virtualsize").
*/
- if (groups[1] && !STREQ(groups[1], "") && (groups[1][0] != '[')) {
- if (virAsprintf(&vol->backingStore.path, "%s/%s",
- pool->def->target.path, groups[1]) < 0) {
- virReportOOMError();
+ if (groups[1] && STRNEQ(groups[1], "") && (groups[1][0] != '[')) {
+ vol->backingStore.path = virStorageBackendGetVolPath(pool->def->source.name,
+ groups[1]);
+ if (!vol->backingStore.path)
goto cleanup;
- }
vol->backingStore.format = VIR_STORAGE_POOL_LOGICAL_LVM2;
}
@@ -277,21 +304,21 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
virStorageVolDefPtr vol)
{
/*
- * # lvs --separator , --noheadings --units b --unbuffered --nosuffix --options \
- * "lv_name,origin,uuid,devices,seg_size,vg_extent_size,size,lv_attr" VGNAME
+ * # lvs --separator '#' --noheadings --units b --unbuffered --nosuffix --options \
+ * "lv_name,origin,uuid,devices,seg_size,vg_extent_size,size,lv_attr,lv_path" VGNAME
*
- * RootLV,,06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky,/dev/hda2(0),5234491392,33554432,5234491392,-wi-ao
- * SwapLV,,oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M,/dev/hda2(156),1040187392,33554432,1040187392,-wi-ao
- * Test2,,3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR,/dev/hda2(219),1073741824,33554432,1073741824,owi-a-
- * Test3,,UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht,/dev/hda2(251),2181038080,33554432,2181038080,-wi-a-
- * Test3,Test2,UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht,/dev/hda2(187),1040187392,33554432,1040187392,swi-a-
+ * RootLV##06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky#/dev/hda2(0)#5234491392#33554432#5234491392#-wi-ao#/dev/VGNAME/RootLV
+ * SwapLV##oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M#/dev/hda2(156)#1040187392#33554432#1040187392#-wi-ao#/dev/VGNAME/SwapLV
+ * Test2##3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR#/dev/hda2(219)#1073741824#33554432#1073741824#owi-a-#/dev/VGNAME/Test2
+ * Test3##UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(251)#2181038080#33554432#2181038080#-wi-a-#/dev/VGNAME/Test3
+ * Test3#Test2#UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(187)#1040187392#33554432#1040187392#swi-a-#/dev/VGNAME/Test3
*
* Pull out name, origin, & uuid, device, device extent start #,
* segment size, extent size, size, attrs
*
* NB can be multiple rows per volume if they have many extents
*
- * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing "," on each line
+ * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing separator on each line
*
* NB Encrypted logical volumes can print ':' in their name, so it is
* not a suitable separator (rhbz 470693).
@@ -299,7 +326,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
* striped, so "," is not a suitable separator either (rhbz 727474).
*/
const char *regexes[] = {
- "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)#(\\S+)#([0-9]+)#(\\S+)#([0-9]+)#([0-9]+)#(\\S+)#?\\s*$"
+ "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)#(\\S+)#([0-9]+)#(\\S+)#([0-9]+)#([0-9]+)#(\\S+)#(\\S+)#?\\s*$"
};
int vars[] = {
10
@@ -314,7 +341,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
"--unbuffered",
"--nosuffix",
"--options",
- "lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size,size,lv_attr",
+ "lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size,size,lv_attr,lv_path",
pool->def->source.name,
NULL);
if (virStorageBackendRunProgRegex(pool,
@@ -470,18 +497,7 @@ virStorageBackendLogicalCheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
virStoragePoolObjPtr pool,
bool *isActive)
{
- char *path;
-
- *isActive = false;
- if (virAsprintf(&path, "/dev/%s", pool->def->source.name) < 0) {
- virReportOOMError();
- return -1;
- }
-
- if (access(path, F_OK) == 0)
- *isActive = true;
-
- VIR_FREE(path);
+ *isActive = (access(pool->def->target.path, F_OK) == 0);
return 0;
}
@@ -702,6 +718,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
int fd = -1;
virCommandPtr cmd = NULL;
virErrorPtr err;
+ bool created = false;
if (vol->target.encryption != NULL) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -717,13 +734,6 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
VIR_FREE(vol->target.path);
}
- if (virAsprintf(&vol->target.path, "%s/%s",
- pool->def->target.path,
- vol->name) == -1) {
- virReportOOMError();
- return -1;
- }
-
cmd = virCommandNewArgList(LVCREATE,
"--name", vol->name,
NULL);
@@ -742,9 +752,15 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
if (virCommandRun(cmd, NULL) < 0)
goto error;
+ created = true;
virCommandFree(cmd);
cmd = NULL;
+ vol->target.path = virStorageBackendGetVolPath(pool->def->source.name,
+ vol->name);
+ if (!vol->target.path)
+ goto error;
+
if ((fd = virStorageBackendVolOpen(vol->target.path)) < 0)
goto error;
@@ -784,7 +800,8 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
error:
err = virSaveLastError();
VIR_FORCE_CLOSE(fd);
- virStorageBackendLogicalDeleteVol(conn, pool, vol, 0);
+ if (created)
+ virStorageBackendLogicalDeleteVol(conn, pool, vol, 0);
virCommandFree(cmd);
virSetError(err);
virFreeError(err);
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index cc8eaa9..891097f 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1,7 +1,7 @@
/*
* storage_driver.c: core driver for storage APIs
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
diff --git a/tests/storagepoolxml2xmlin/pool-logical-create.xml b/tests/storagepoolxml2xmlin/pool-logical-create.xml
index 4c67089..fd551e0 100644
--- a/tests/storagepoolxml2xmlin/pool-logical-create.xml
+++ b/tests/storagepoolxml2xmlin/pool-logical-create.xml
@@ -10,7 +10,7 @@
<device path="/dev/sdb3"/>
</source>
<target>
- <path>/dev/HostVG</path>
+ <path>/invalid/nonexistent/path</path>
<permissions>
<mode>0700</mode>
<owner>0</owner>
diff --git a/tests/storagepoolxml2xmlin/pool-logical-nopath.xml b/tests/storagepoolxml2xmlin/pool-logical-nopath.xml
new file mode 100644
index 0000000..307e2ab
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-logical-nopath.xml
@@ -0,0 +1,18 @@
+<pool type='logical'>
+ <name>HostVG</name>
+ <uuid>1c13165a-d0f4-3aee-b447-30fb38789091</uuid>
+ <capacity>99891544064</capacity>
+ <allocation>99220455424</allocation>
+ <available>671088640</available>
+ <source>
+ <name>HostVG</name>
+ <format type='lvm2'/>
+ </source>
+ <target>
+ <permissions>
+ <mode>0700</mode>
+ <owner>0</owner>
+ <group>0</group>
+ </permissions>
+ </target>
+</pool>
diff --git a/tests/storagepoolxml2xmlout/pool-logical-nopath.xml b/tests/storagepoolxml2xmlout/pool-logical-nopath.xml
new file mode 100644
index 0000000..07860ef
--- /dev/null
+++ b/tests/storagepoolxml2xmlout/pool-logical-nopath.xml
@@ -0,0 +1,19 @@
+<pool type='logical'>
+ <name>HostVG</name>
+ <uuid>1c13165a-d0f4-3aee-b447-30fb38789091</uuid>
+ <capacity unit='bytes'>0</capacity>
+ <allocation unit='bytes'>0</allocation>
+ <available unit='bytes'>0</available>
+ <source>
+ <name>HostVG</name>
+ <format type='lvm2'/>
+ </source>
+ <target>
+ <path>/dev/HostVG</path>
+ <permissions>
+ <mode>0700</mode>
+ <owner>0</owner>
+ <group>0</group>
+ </permissions>
+ </target>
+</pool>
diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
index 0376e63..3becdcf 100644
--- a/tests/storagepoolxml2xmltest.c
+++ b/tests/storagepoolxml2xmltest.c
@@ -85,6 +85,7 @@ mymain(void)
DO_TEST("pool-dir");
DO_TEST("pool-fs");
DO_TEST("pool-logical");
+ DO_TEST("pool-logical-nopath");
DO_TEST("pool-logical-create");
DO_TEST("pool-disk");
DO_TEST("pool-iscsi");
--
1.8.2.1
12 years
[libvirt] Design of virDomainSnapshotExport/Import
by Xu Wang
Applications based on libvirt such as OpenStack need some public APIs
about export/import of external snapshot during daily backup, snapshot
recovery or system migration. virDomainSnapshotDiskExport,
virDomainSnapshotVmstateExport and virDomainSnapshotImport are designed
to satisfy such requirement.
@flags: virDomainSnapshotExportImportFlags include,
DATA_FULL: all data snapshot
DATA_DELTA: delta data only
/**
* virDomainSnapshotDiskExport
* @domain: the domain object to be exported snapshot
* @id: top Overlays which to be exported
* @base_id: RootBase of snapshot to be exported
* @disk: block device of domain
* @export_path: path of snapshots export
* @flags: bitwise-OR of virDomainSnapshotExportImportFlags
*
* Import snapshot disk data to the pointed path.
*
* If @flags includes DATA_FULL, full backup snapshot will be exported.
*
* If @flags includes DATA_DELTA, incremental backup snapshot will be
* exported.
*
* DATA_FULL and DATA_DELTA should not appear at the same time
*
* Returns 0 on success, or -1 on error
*/
int virDomainSnapshotDiskExport(virDomain *domain,
virSnapshotId *id,
virSnapshotId *base_id,
virDevice *disk,
char *export_path,
unsigned int flags)
Usage: export external snapshot disk file from virtual machine to the
target path.
/**
* virDomainSnapshotVmstateExport
* @domain: the domain object to be exported snapshot
* @id: top Overlays which to be exported
* @export_path: path of snapshots export
* @flags: not used yet, so callers should always pass 0
*
* Import snapshot vmstate data to the pointed path.
*
* If @flags includes DATA_FULL, full backup snapshot will be exported.
*
* If @flags includes DATA_DELTA, incremental backup snapshot will be
* exported.
*
* DATA_FULL and DATA_DELTA should not appear at the same time
*
* Returns 0 on success, or -1 on error
*/
int virDomainSnapshotVmstateExport(virDomain *domain,
virSnapshotId *id,
char *export_path,
unsigned int flags)
Usage: dump vmstate file of snapshot and copy it to the target path.
/**
* virDomainSnapshotImport
* @domain: the domain object to be exported snapshot
* @xmlDesc: string containing an XML description of the domain
* @flags: bitwise-OR of virDomainSnapshotExportImportFlags
*
* Import snapshot data to the pointed path.
*
* If @flags includes DATA_FULL, full backup snapshot will be exported.
*
* If @flags includes DATA_DELTA, incremental backup snapshot will be
* exported.
*
* DATA_FULL and DATA_DELTA should not appear at the same time
*
* Returns 0 on success, or -1 on error
*/
int virDomainSnapshotImport(virDomain *domain,
const char *xmlDesc,
unsigned int flags)
Usage: import .xml file of external snapshot and use it for recovering
a virtual machine.
Usecase:
If users want to finish daily backup&recovery work by above APIs,
the process should be,
1. call virDomainSnapshotDiskExport() to export pointed snapshot
disk files to the traget path.
------------------- ---------------------
| host A | | host B |
| --------------- | | ----------------- |
| | dom1 | | snapshot disk files | | backup folder | |
| | base.img----------------------------------->base.img | |
| | snap1.qcow2-------------------------------->snap1.qcow2 | |
| | snap2.qcow2-------------------------------->snap2.qcow2 | |
| --------------- | | ----------------- |
------------------- ---------------------
virDomainSnapshotDiskSnapshot()
2. call virDomainSnapshotVmstateExport() to export vmstate data
file to the target path.
--------------- ---------------------
| host A | | host B |
| ----------- | | ----------------- |
| | dom1 | | vmstate data files | | backup folder | |
| | dumpxml-------------------------------->vmstate.xml | |
| ----------- | | ----------------- |
--------------- ---------------------
virDomainSnapshotVmstateSnapshot()
3. when some error occured, or guest copy/migration, users copy
snapshot disk files from host B to the target host and edit <source
file> field of <disk> tag. Then users call virDomainSnapshotImport
to recovery domain to the snapshot.
a. copy snapshot disk files to the target host:
------------------------------- ---------------------
| host A | snapshot | host B |
| --------------------------- | disk | ----------------- |
| | /var/lib/libvirt/images | | files | | backup folder | |
| | base.img<-----------------------------------base.img | |
| | snap1.qcow2<--------------------------------snap1.qcow2 | |
| | snap2.qcow2<--------------------------------snap2.qcow2 | |
| --------------------------- | | ----------------- |
------------------------------- ---------------------
snapshot disk files transfer
b. import vmstate data to recovery snapshot:
---------------------
------------------- | host B |
| host A | | ----------------- |
| --------------- | vmstate data | | backup folder | |
| | dom1<-------------------------------vmstate.xml | |
| | base.img | | | ----------------- |
| | snap1.qcow2 | | ---------------------
| | snap2.qcow2 | |
| --------------- |
-------------------
virDomainSnapshotImport()
12 years
[libvirt] [PATCH] util: fix build error on non-Linux systems
by Laine Stump
Building on FreeBSD had this linker error:
/work/a/ports/devel/libvirt/work/libvirt-1.1.0/src/.libs/libvirt.so:
undefined reference to `virPCIDeviceAddressParse'
This was caused by the new use of virPCIDeviceAddressParse in a
portion of virpci.c that wasn't linux-only (in commit 72c029d8). The
problem was that virPCIDeviceAddressParse had originally been defined
inside #ifdef _linux (because it was only used by another function
that was inside the same ifdef).
The solution is to move it out to the part of virpci.c that is
compiled on all platforms.
(Because the portion that was "moved" was 40-50 lines, but only moved
up by 15 lines, the diff for the patch is less than non-informative -
rather than showing that part that I moved, it shows the bit that was
previously before the moved part, and now sits *after* it.)
---
Pushed under the build-breaker rule.
I actually hadn't noticed that some of the functions in virpci.c were
Linux-only. It looks like this was done because those functions use
the Linux sysfs to get information about the devices. However, much of
the rest of virpci.c does the same thing, so we should probably
revisit which of the functions in that file are compiled only on Linux
and which are compile for everyone.
src/util/virpci.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 0ed29e7..7d83bdb 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2261,21 +2261,6 @@ int virPCIDeviceIsAssignable(virPCIDevicePtr dev,
return 1;
}
-#ifdef __linux__
-
-/*
- * returns true if equal
- */
-static bool
-virPCIDeviceAddressIsEqual(virPCIDeviceAddressPtr bdf1,
- virPCIDeviceAddressPtr bdf2)
-{
- return ((bdf1->domain == bdf2->domain) &&
- (bdf1->bus == bdf2->bus) &&
- (bdf1->slot == bdf2->slot) &&
- (bdf1->function == bdf2->function));
-}
-
static int
logStrToLong_ui(char const *s,
char **end_ptr,
@@ -2327,6 +2312,21 @@ out:
return ret;
}
+#ifdef __linux__
+
+/*
+ * returns true if equal
+ */
+static bool
+virPCIDeviceAddressIsEqual(virPCIDeviceAddressPtr bdf1,
+ virPCIDeviceAddressPtr bdf2)
+{
+ return ((bdf1->domain == bdf2->domain) &&
+ (bdf1->bus == bdf2->bus) &&
+ (bdf1->slot == bdf2->slot) &&
+ (bdf1->function == bdf2->function));
+}
+
static int
virPCIGetDeviceAddressFromSysfsLink(const char *device_link,
virPCIDeviceAddressPtr *bdf)
--
1.7.11.7
12 years
[libvirt] Restore original security labels
by Michal Privoznik
Dear list,
it's been a while since I've tried to get the patches in [1].
However, it turned out that we need completely different approach. Now
I'd like to revisit that decision.
The problem is: libvirt sets various security labels (DAC, selinux) in
order for a file to be readable by a qemu process. However, it doesn't
record the original labels, so in process of tearing the domain down, we
restore "defaults" (in case of DAC we set root:root instead of
john:doe). Moreover, if a file is to be shared among multiple domains we
can't restore the label as it would make it inaccessible for other qemu
processes.
My implementation dealt with this problem using XATTRs: one to store the
original label, the other one as a reference counter. For each labeling
the counter is increased. For each attempt to restore the label the
counter is decreased. The original label is restored iff the counter is
zero. However, this approach doesn't work well with two libvirtd
instances fighting over a file. But one can argue that this is something
for cluster. The question is - do we want to reimplement cluster in libvirt?
I think my approach seems like reasonable trade-off. So what's your
opinion on this?
Michal
1: http://www.redhat.com/archives/libvir-list/2013-March/msg01289.html
12 years
[libvirt] [PATCH] add symbol ")" in virsh nodedev-detach help
by xuzhang
---
tools/virsh-nodedev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index da93b8e..49546a2 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -605,7 +605,7 @@ static const vshCmdOptDef opts_node_device_detach[] = {
},
{.name = "driver",
.type = VSH_OT_STRING,
- .help = N_("pci device assignment backend driver (e.g. 'vfio' or 'kvm'")
+ .help = N_("pci device assignment backend driver (e.g. 'vfio' or 'kvm'"))
},
{.name = NULL}
};
--
1.7.11.7
12 years
[libvirt] [PATCH] Make logical pools independent on target path
by Martin Kletzander
When using logical pools, we had to trust the target->pth provided.
This parameter, however, can be completely ommited and we can get the
correct path using 'lvdisplay' command.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=952973
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
Notes:
I'm not sure we can drop the target.path from the XML (see tests), so
another variant is to keep it there the same way it was defined by
user/mgmt app. If that's desired, mentioning it with an ack is enough
for me to know I should drop the conf/storage_conf.c hunk as well as
the hunks patching tests.
configure.ac | 4 ++
src/conf/storage_conf.c | 19 +++---
src/storage/storage_backend_logical.c | 79 +++++++++++++++-------
tests/storagepoolxml2xmlin/pool-logical-create.xml | 1 -
tests/storagepoolxml2xmlin/pool-logical.xml | 1 -
.../storagepoolxml2xmlout/pool-logical-create.xml | 1 -
tests/storagepoolxml2xmlout/pool-logical.xml | 1 -
7 files changed, 67 insertions(+), 39 deletions(-)
diff --git a/configure.ac b/configure.ac
index 5d1bc6b..753139d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1603,6 +1603,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then
AC_PATH_PROG([PVS], [pvs], [], [$PATH:/sbin:/usr/sbin])
AC_PATH_PROG([VGS], [vgs], [], [$PATH:/sbin:/usr/sbin])
AC_PATH_PROG([LVS], [lvs], [], [$PATH:/sbin:/usr/sbin])
+ AC_PATH_PROG([LVDISPLAY], [lvdisplay], [], [$PATH:/sbin:/usr/sbin])
if test "$with_storage_lvm" = "yes" ; then
if test -z "$PVCREATE" ; then AC_MSG_ERROR([We need pvcreate for LVM storage driver]) ; fi
@@ -1617,6 +1618,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then
if test -z "$PVS" ; then AC_MSG_ERROR([We need pvs for LVM storage driver]) ; fi
if test -z "$VGS" ; then AC_MSG_ERROR([We need vgs for LVM storage driver]) ; fi
if test -z "$LVS" ; then AC_MSG_ERROR([We need lvs for LVM storage driver]) ; fi
+ if test -z "$LVDISPLAY" ; then AC_MSG_ERROR([We need lvdisplay for LVM storage driver]) ; fi
else
if test -z "$PVCREATE" ; then with_storage_lvm=no ; fi
if test -z "$VGCREATE" ; then with_storage_lvm=no ; fi
@@ -1630,6 +1632,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then
if test -z "$PVS" ; then with_storage_lvm=no ; fi
if test -z "$VGS" ; then with_storage_lvm=no ; fi
if test -z "$LVS" ; then with_storage_lvm=no ; fi
+ if test -z "$LVDISPLAY" ; then with_storage_lvm=no ; fi
if test "$with_storage_lvm" = "check" ; then with_storage_lvm=yes ; fi
fi
@@ -1648,6 +1651,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then
AC_DEFINE_UNQUOTED([PVS],["$PVS"],[Location of pvs program])
AC_DEFINE_UNQUOTED([VGS],["$VGS"],[Location of vgs program])
AC_DEFINE_UNQUOTED([LVS],["$LVS"],[Location of lvs program])
+ AC_DEFINE_UNQUOTED([LVDISPLAY],["$LVDISPLAY"],[Location of lvdisplay program])
fi
fi
AM_CONDITIONAL([WITH_STORAGE_LVM], [test "$with_storage_lvm" = "yes"])
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index cc3d3d9..948e190 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1,7 +1,7 @@
/*
* storage_conf.c: config handling for storage driver
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -947,15 +947,16 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
/* When we are working with a virtual disk we can skip the target
* path and permissions */
if (!(options->flags & VIR_STORAGE_POOL_SOURCE_NETWORK)) {
- if (!(target_path = virXPathString("string(./target/path)", ctxt))) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing storage pool target path"));
- goto error;
+ if (ret->type != VIR_STORAGE_POOL_LOGICAL) {
+ if (!(target_path = virXPathString("string(./target/path)", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing storage pool target path"));
+ goto error;
+ }
+ ret->target.path = virFileSanitizePath(target_path);
+ if (!ret->target.path)
+ goto error;
}
- ret->target.path = virFileSanitizePath(target_path);
- if (!ret->target.path)
- goto error;
-
if (virStorageDefParsePerms(ctxt, &ret->target.perms,
"./target/permissions",
DEFAULT_POOL_PERM_MODE) < 0)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index 944aa0e..66a4e42 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -45,6 +45,37 @@
#define PV_BLANK_SECTOR_SIZE 512
+static char *
+virStorageBackendGetVolPath(const char *poolname, const char *volname)
+{
+ char *start = NULL;
+ char *lvpath = NULL;
+ char *output = NULL;
+
+ virCommandPtr cmd = virCommandNewArgList(LVDISPLAY,
+ "--columns",
+ "--options", "lv_path",
+ "--noheadings",
+ "--unbuffered",
+ NULL);
+
+ virCommandAddArgFormat(cmd, "%s/%s", poolname, volname);
+ virCommandSetOutputBuffer(cmd, &output);
+
+ if (virCommandRun(cmd, NULL) < 0 ||
+ !(start = strchr(output, '/'))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot get LV path"));
+ goto cleanup;
+ }
+
+ ignore_value(VIR_STRDUP(lvpath, start));
+
+ cleanup:
+ VIR_FREE(output);
+ virCommandFree(cmd);
+ return lvpath;
+}
+
static int
virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
int on)
@@ -116,11 +147,8 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
}
if (vol->target.path == NULL) {
- if (virAsprintf(&vol->target.path, "%s/%s",
- pool->def->target.path, vol->name) < 0) {
- virReportOOMError();
+ if (VIR_STRDUP(vol->target.path, groups[9]) < 0)
goto cleanup;
- }
}
/* Skips the backingStore of lv created with "--virtualsize",
@@ -130,12 +158,11 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
* (lvs outputs "[$lvname_vorigin] for field "origin" if the
* lv is created with "--virtualsize").
*/
- if (groups[1] && !STREQ(groups[1], "") && (groups[1][0] != '[')) {
- if (virAsprintf(&vol->backingStore.path, "%s/%s",
- pool->def->target.path, groups[1]) < 0) {
- virReportOOMError();
+ if (groups[1] && STRNEQ(groups[1], "") && (groups[1][0] != '[')) {
+ vol->backingStore.path = virStorageBackendGetVolPath(pool->def->source.name,
+ groups[1]);
+ if (!vol->backingStore.path)
goto cleanup;
- }
vol->backingStore.format = VIR_STORAGE_POOL_LOGICAL_LVM2;
}
@@ -277,21 +304,20 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
virStorageVolDefPtr vol)
{
/*
- * # lvs --separator , --noheadings --units b --unbuffered --nosuffix --options \
- * "lv_name,origin,uuid,devices,seg_size,vg_extent_size,size,lv_attr" VGNAME
+ * # lvs --separator '#' --noheadings --units b --unbuffered --nosuffix --options "lv_name,origin,uuid,devices,seg_size,vg_extent_size,size,lv_path" VGNAME
*
- * RootLV,,06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky,/dev/hda2(0),5234491392,33554432,5234491392,-wi-ao
- * SwapLV,,oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M,/dev/hda2(156),1040187392,33554432,1040187392,-wi-ao
- * Test2,,3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR,/dev/hda2(219),1073741824,33554432,1073741824,owi-a-
- * Test3,,UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht,/dev/hda2(251),2181038080,33554432,2181038080,-wi-a-
- * Test3,Test2,UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht,/dev/hda2(187),1040187392,33554432,1040187392,swi-a-
+ * RootLV##06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky#/dev/hda2(0)#5234491392#33554432#5234491392#/dev/VGNAME/RootLV
+ * SwapLV##oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M#/dev/hda2(156)#1040187392#33554432#1040187392#/dev/VGNAME/SwapLV
+ * Test2##3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR#/dev/hda2(219)#1073741824#33554432#1073741824#/dev/VGNAME/Test2
+ * Test3##UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(251)#2181038080#33554432#2181038080#/dev/VGNAME/Test3
+ * Test3#Test2#UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(187)#1040187392#33554432#1040187392#/dev/VGNAME/Test3
*
* Pull out name, origin, & uuid, device, device extent start #,
* segment size, extent size, size, attrs
*
* NB can be multiple rows per volume if they have many extents
*
- * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing "," on each line
+ * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing separator on each line
*
* NB Encrypted logical volumes can print ':' in their name, so it is
* not a suitable separator (rhbz 470693).
@@ -314,7 +340,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
"--unbuffered",
"--nosuffix",
"--options",
- "lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size,size,lv_attr",
+ "lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size,size,lv_path",
pool->def->source.name,
NULL);
if (virStorageBackendRunProgRegex(pool,
@@ -702,6 +728,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
int fd = -1;
virCommandPtr cmd = NULL;
virErrorPtr err;
+ bool created = false;
if (vol->target.encryption != NULL) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -717,13 +744,6 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
VIR_FREE(vol->target.path);
}
- if (virAsprintf(&vol->target.path, "%s/%s",
- pool->def->target.path,
- vol->name) == -1) {
- virReportOOMError();
- return -1;
- }
-
cmd = virCommandNewArgList(LVCREATE,
"--name", vol->name,
NULL);
@@ -742,9 +762,15 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
if (virCommandRun(cmd, NULL) < 0)
goto error;
+ created = true;
virCommandFree(cmd);
cmd = NULL;
+ vol->target.path = virStorageBackendGetVolPath(pool->def->source.name,
+ vol->name);
+ if (!vol->target.path)
+ goto error;
+
if ((fd = virStorageBackendVolOpen(vol->target.path)) < 0)
goto error;
@@ -784,7 +810,8 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
error:
err = virSaveLastError();
VIR_FORCE_CLOSE(fd);
- virStorageBackendLogicalDeleteVol(conn, pool, vol, 0);
+ if (created)
+ virStorageBackendLogicalDeleteVol(conn, pool, vol, 0);
virCommandFree(cmd);
virSetError(err);
virFreeError(err);
diff --git a/tests/storagepoolxml2xmlin/pool-logical-create.xml b/tests/storagepoolxml2xmlin/pool-logical-create.xml
index 4c67089..e1bb4db 100644
--- a/tests/storagepoolxml2xmlin/pool-logical-create.xml
+++ b/tests/storagepoolxml2xmlin/pool-logical-create.xml
@@ -10,7 +10,6 @@
<device path="/dev/sdb3"/>
</source>
<target>
- <path>/dev/HostVG</path>
<permissions>
<mode>0700</mode>
<owner>0</owner>
diff --git a/tests/storagepoolxml2xmlin/pool-logical.xml b/tests/storagepoolxml2xmlin/pool-logical.xml
index c4bfa07..307e2ab 100644
--- a/tests/storagepoolxml2xmlin/pool-logical.xml
+++ b/tests/storagepoolxml2xmlin/pool-logical.xml
@@ -9,7 +9,6 @@
<format type='lvm2'/>
</source>
<target>
- <path>/dev/HostVG</path>
<permissions>
<mode>0700</mode>
<owner>0</owner>
diff --git a/tests/storagepoolxml2xmlout/pool-logical-create.xml b/tests/storagepoolxml2xmlout/pool-logical-create.xml
index 7413f1c..fbce90e 100644
--- a/tests/storagepoolxml2xmlout/pool-logical-create.xml
+++ b/tests/storagepoolxml2xmlout/pool-logical-create.xml
@@ -12,7 +12,6 @@
<format type='lvm2'/>
</source>
<target>
- <path>/dev/HostVG</path>
<permissions>
<mode>0700</mode>
<owner>0</owner>
diff --git a/tests/storagepoolxml2xmlout/pool-logical.xml b/tests/storagepoolxml2xmlout/pool-logical.xml
index 07860ef..7fad0d8 100644
--- a/tests/storagepoolxml2xmlout/pool-logical.xml
+++ b/tests/storagepoolxml2xmlout/pool-logical.xml
@@ -9,7 +9,6 @@
<format type='lvm2'/>
</source>
<target>
- <path>/dev/HostVG</path>
<permissions>
<mode>0700</mode>
<owner>0</owner>
--
1.8.2.1
12 years
[libvirt] [PATCH] add symbol ")" in virsh nodedev-detach help
by xuzhang
---
tools/virsh-nodedev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index da93b8e..49546a2 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -605,7 +605,7 @@ static const vshCmdOptDef opts_node_device_detach[] = {
},
{.name = "driver",
.type = VSH_OT_STRING,
- .help = N_("pci device assignment backend driver (e.g. 'vfio' or 'kvm'")
+ .help = N_("pci device assignment backend driver (e.g. 'vfio' or 'kvm'"))
},
{.name = NULL}
};
--
1.7.11.7
12 years
[libvirt] [PATCH 0/2] Fix a pair of network-related crashes
by Ján Tomko
Ján Tomko (2):
netdev: accept NULL in virNetDevSetupControl
bridge: don't crash on bandwidth unplug with no bandwidth
src/network/bridge_driver.c | 5 +++++
src/util/virnetdev.c | 14 ++++++++------
2 files changed, 13 insertions(+), 6 deletions(-)
--
1.8.1.5
12 years
[libvirt] [PATCH 0/5]New atomic API to delete snapshot object
by Guannan Ren
Add a new snapshot API to delete snapshot object atomically
int virDomainSnapshotDeleteByName(virDomainPtr domain,
const char *name,
unsigned int flags);
The existing virDomainSnapshotDelete API accepts the snapshot
object being deleted as an argument that would be not API atomic.
Guannan Ren(5)
[PATCH 1/5] snapshot: define new API virDomainSnapshotDeleteByName
[PATCH 2/5] snapshot: auto generate RPC calls for remoteDomainSnapshotDeleteByName
[PATCH 3/5] qemu: implement SnapshotDeleteByName
[PATCH 4/5] python: make auto-generated function name nicer
[PATCH 5/5] virsh: use virDomainSnapshotDeleteByName in virsh
include/libvirt/libvirt.h.in | 5 +++++
python/generator.py | 3 +++
src/driver.h | 6 ++++++
src/libvirt.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 5 +++++
src/qemu/qemu_driver.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 13 +++++++++++-
src/remote_protocol-structs | 7 +++++++
tools/virsh-snapshot.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++----------
10 files changed, 249 insertions(+), 30 deletions(-)
12 years
