6:19 p.m.
v2 to v3: not only check disk source, startupPolicy should work if any
backing file is missing. The commit 039a3283 break the limition
of contiguous device boot orders, so I remove my previous patch
about it.
v1 to v2: added relax schema for disk of block and dir type
removed original patch 3/5.
The set of patches is trying to add 'startupPolicy' attribute support
to the source element of hard disks. Policy levels are using the
mandatory, requisite, optional levels as originally documented.
For the 'optional' policy, there is a little difference from CDROM and
Floppy which only drop its source path, for disks, if missing, the
checking function will drop their definitions, because qemu doesn't
allow missing source path for hard disk.
Guannan Ren
[PATCH v3 1/4] conf: add startupPolicy attribute for harddisk
[PATCH v3 2/4] storage: report error rather than warning if backing files doesn't
exist
[PATCH v3 3/4] qemu: drop disk with 'optional' startupPolicy if it is not
present
[PATCH v3 4/4] security: restore DAC for every disk file in disk chain
docs/schemas/domaincommon.rng | 6 +++++
include/libvirt/libvirt.h.in | 1 +
src/conf/domain_conf.c | 20 ++++++++++------
src/qemu/qemu_domain.c | 119
++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------
src/qemu/qemu_process.c | 7 ------
src/security/security_dac.c | 15 +++++++++++-
src/util/virstoragefile.c | 23 +++++++++++-------
tests/virstoragetest.c | 16 ++++++-------
8 files changed, 130 insertions(+), 77 deletions(-)
6:19 p.m.
New subject: [libvirt] [PATCH v3 1/4] conf: add startupPolicy attribute for harddisk
Add startupPolicy attribute policy for harddisk with type "file",
"block" and "dir". The "network" type disk is still not
supported.
---
docs/schemas/domaincommon.rng | 6 ++++++
src/conf/domain_conf.c | 20 +++++++++++++-------
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3cace35..fb3f980 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1095,6 +1095,9 @@
<ref name="absFilePath"/>
</attribute>
<optional>
+ <ref name="startupPolicy"/>
+ </optional>
+ <optional>
<ref name='devSeclabel'/>
</optional>
</element>
@@ -1112,6 +1115,9 @@
<attribute name="dir">
<ref name="absFilePath"/>
</attribute>
+ <optional>
+ <ref name="startupPolicy"/>
+ </optional>
<empty/>
</element>
</optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3e81013..4485756 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4737,7 +4737,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
switch (def->type) {
case VIR_DOMAIN_DISK_TYPE_FILE:
source = virXMLPropString(cur, "file");
- startupPolicy = virXMLPropString(cur, "startupPolicy");
break;
case VIR_DOMAIN_DISK_TYPE_BLOCK:
source = virXMLPropString(cur, "dev");
@@ -4826,7 +4825,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
case VIR_DOMAIN_DISK_TYPE_VOLUME:
if (virDomainDiskSourcePoolDefParse(cur, def) < 0)
goto error;
- startupPolicy = virXMLPropString(cur, "startupPolicy");
break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -4835,6 +4833,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
}
+ startupPolicy = virXMLPropString(cur, "startupPolicy");
+
/* People sometimes pass a bogus '' source path
when they mean to omit the source element
completely (e.g. CDROM without media). This is
@@ -5400,11 +5400,10 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
}
- if (def->device != VIR_DOMAIN_DISK_DEVICE_CDROM &&
- def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+ if (def->type == VIR_DOMAIN_DISK_TYPE_NETWORK) {
virReportError(VIR_ERR_INVALID_ARG,
- _("Setting disk %s is allowed only for "
- "cdrom or floppy"),
+ _("Setting disk %s is not allowed for "
+ "disk of network type"),
startupPolicy);
goto error;
}
@@ -13805,6 +13804,9 @@ virDomainDiskSourceDefFormat(virBufferPtr buf,
case VIR_DOMAIN_DISK_TYPE_BLOCK:
virBufferEscapeString(buf, " <source dev='%s'",
def->src);
+ if (def->startupPolicy)
+ virBufferEscapeString(buf, " startupPolicy='%s'",
+ startupPolicy);
if (def->nseclabels) {
virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 8);
@@ -13817,8 +13819,12 @@ virDomainDiskSourceDefFormat(virBufferPtr buf,
}
break;
case VIR_DOMAIN_DISK_TYPE_DIR:
- virBufferEscapeString(buf, " <source
dir='%s'/>\n",
+ virBufferEscapeString(buf, " <source dir='%s'",
def->src);
+ if (def->startupPolicy)
+ virBufferEscapeString(buf, " startupPolicy='%s'",
+ startupPolicy);
+ virBufferAddLit(buf, "/>\n");
break;
case VIR_DOMAIN_DISK_TYPE_NETWORK:
virBufferAsprintf(buf, " <source protocol='%s'",
--
1.8.1.4
6:19 p.m.
New subject: [libvirt] [PATCH v3 2/4] storage: report error rather than warning if backing files doesn't exist
If one of backing files for disk source doesn't exist, the guest will not
be able to find and use the disk even though the disk still exists in
guest xml definition. So reporting an error make more sense.
Adding virFileAccessibleAs() to check if the backing file described in
disk meta exist in real path. If not, report error. the uid and gid
arguments don't have so much meannings for F_OK, so give 0 for them.
---
src/util/virstoragefile.c | 23 +++++++++++++++--------
tests/virstoragetest.c | 16 ++++++++--------
2 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index b985df4..4781221 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -534,6 +534,13 @@ virFindBackingFile(const char *start, bool start_is_dir, const char
*path,
goto cleanup;
}
+ if (virFileAccessibleAs(combined, F_OK, 0, 0) < 0) {
+ virReportSystemError(errno,
+ _("Backing file '%s' does not exist"),
+ combined);
+ goto cleanup;
+ }
+
if (!(*canonical = canonicalize_file_name(combined))) {
virReportSystemError(errno,
_("Can't canonicalize path '%s'"),
path);
@@ -784,14 +791,10 @@ virStorageFileGetMetadataInternal(const char *path,
!!directory, backing,
&meta->directory,
&meta->backingStore) < 0) {
- /* the backing file is (currently) unavailable, treat this
- * file as standalone:
- * backingStoreRaw is kept to mark broken image chains */
- meta->backingStoreIsFile = false;
- backingFormat = VIR_STORAGE_FILE_NONE;
- VIR_WARN("Backing file '%s' of image '%s' is
missing.",
- meta->backingStoreRaw, path);
-
+ VIR_ERROR(_("Backing file '%s' of image '%s' is
missing."),
+ meta->backingStoreRaw, path);
+ VIR_FREE(backing);
+ goto cleanup;
}
}
VIR_FREE(backing);
@@ -968,6 +971,10 @@ virStorageFileGetMetadataRecurse(const char *path, const char
*directory,
uid, gid,
allow_probe,
cycle);
+ if (!ret->backingMeta) {
+ virStorageFileFreeMetadata(ret);
+ ret = NULL;
+ }
}
return ret;
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index fef4b37..fb5da5c 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -483,10 +483,10 @@ mymain(void)
/* Qcow2 file with missing backing file but specified type */
const testFileData chain9[] = { qcow2_bogus };
TEST_CHAIN(9, "qcow2", absqcow2, VIR_STORAGE_FILE_QCOW2,
- chain9, EXP_WARN,
- chain9, ALLOW_PROBE | EXP_WARN,
- chain9, EXP_WARN,
- chain9, ALLOW_PROBE | EXP_WARN);
+ chain9, EXP_FAIL,
+ chain9, ALLOW_PROBE | EXP_FAIL,
+ chain9, EXP_FAIL,
+ chain9, ALLOW_PROBE | EXP_FAIL);
/* Rewrite qcow2 to a missing backing file, without backing type */
virCommandFree(cmd);
@@ -498,10 +498,10 @@ mymain(void)
/* Qcow2 file with missing backing file and no specified type */
const testFileData chain10[] = { qcow2_bogus };
TEST_CHAIN(10, "qcow2", absqcow2, VIR_STORAGE_FILE_QCOW2,
- chain10, EXP_WARN,
- chain10, ALLOW_PROBE | EXP_WARN,
- chain10, EXP_WARN,
- chain10, ALLOW_PROBE | EXP_WARN);
+ chain10, EXP_FAIL,
+ chain10, ALLOW_PROBE | EXP_FAIL,
+ chain10, EXP_FAIL,
+ chain10, ALLOW_PROBE | EXP_FAIL);
/* Rewrite qcow2 to use an nbd: protocol as backend */
virCommandFree(cmd);
--
1.8.1.4
6:19 p.m.
New subject: [libvirt] [PATCH v3 3/4] qemu: drop disk with 'optional' startupPolicy if it is not present
Go through disks of guest, if one disk doesn't exist or its backing
chain is broken, with 'optional' startupPolicy, for CDROM and Floppy
we only discard its source path definition in xml, for disks we drop
it from disk list and free it.
Originally, for these disks without startupPolicy attribute, libvirt
ignore the presence checking, this patch make startupPolicy 'mandatory'
as default.
---
include/libvirt/libvirt.h.in | 1 +
src/qemu/qemu_domain.c | 119 ++++++++++++++++++++++++++-----------------
src/qemu/qemu_process.c | 7 ---
3 files changed, 74 insertions(+), 53 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index e057be1..14d5363 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4610,6 +4610,7 @@ typedef void (*virConnectDomainEventBlockJobCallback)(virConnectPtr
conn,
*/
typedef enum {
VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START = 0, /* oldSrcPath is set */
+ VIR_DOMAIN_EVENT_HARDDISK_DROP_MISSING_ON_START = 1,
#ifdef VIR_ENUM_SENTINELS
VIR_DOMAIN_EVENT_DISK_CHANGE_LAST
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index fdcf7bc..62a54ef 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1977,75 +1977,102 @@ cleanup:
virObjectUnref(cfg);
}
+static int
+qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk)
+{
+ char uuid[VIR_UUID_STRING_BUFLEN];
+ virDomainEventPtr event = NULL;
+ virDomainDiskDefPtr del_disk = NULL;
+
+ virUUIDFormat(vm->def->uuid, uuid);
+
+ VIR_DEBUG("Dropping disk '%s' on domain '%s' (UUID '%s')
"
+ "due to inaccessible source '%s'",
+ disk->dst, vm->def->name, uuid, disk->src);
+
+ if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ||
+ disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+
+ event = virDomainEventDiskChangeNewFromObj(vm, disk->src, NULL,
+ disk->info.alias,
+
VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START);
+
+ VIR_FREE(disk->src);
+ } else {
+ event = virDomainEventDiskChangeNewFromObj(vm, disk->src, NULL,
+ disk->info.alias,
+
VIR_DOMAIN_EVENT_HARDDISK_DROP_MISSING_ON_START);
+
+ if (!(del_disk = virDomainDiskRemoveByName(vm->def, disk->src))) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("no source device %s"), disk->src);
+ return -1;
+ }
+ virDomainDiskDefFree(del_disk);
+ }
+
+ if (event)
+ qemuDomainEventQueue(driver, event);
+
+ return 0;
+}
+
int
qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
virDomainObjPtr vm,
bool cold_boot)
{
int ret = -1;
- int i;
+ size_t i;
virDomainDiskDefPtr disk;
- char uuid[VIR_UUID_STRING_BUFLEN];
- virDomainEventPtr event = NULL;
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-
- virUUIDFormat(vm->def->uuid, uuid);
-
- for (i = 0; i < vm->def->ndisks; i++) {
- disk = vm->def->disks[i];
+ size_t count = vm->def->ndisks;
+ size_t nextdisk = 0;
- if (!disk->startupPolicy || !disk->src)
- continue;
+ for (i = 0; i < count; i++) {
+ disk = vm->def->disks[nextdisk];
- if (virFileAccessibleAs(disk->src, F_OK,
- cfg->user,
- cfg->group) >= 0) {
- /* disk accessible */
+ if (qemuDomainDetermineDiskChain(driver, disk, false) == 0) {
+ nextdisk++;
continue;
}
switch ((enum virDomainStartupPolicy) disk->startupPolicy) {
- case VIR_DOMAIN_STARTUP_POLICY_OPTIONAL:
- break;
+ case VIR_DOMAIN_STARTUP_POLICY_OPTIONAL:
+ break;
- case VIR_DOMAIN_STARTUP_POLICY_MANDATORY:
- virReportSystemError(errno,
- _("cannot access file '%s'"),
- disk->src);
- goto cleanup;
- break;
-
- case VIR_DOMAIN_STARTUP_POLICY_REQUISITE:
- if (cold_boot) {
- virReportSystemError(errno,
- _("cannot access file '%s'"),
- disk->src);
- goto cleanup;
- }
- break;
+ case VIR_DOMAIN_STARTUP_POLICY_REQUISITE:
+ if (cold_boot) {
+ goto error;
+ }
+ break;
+
+ case VIR_DOMAIN_STARTUP_POLICY_MANDATORY:
+ case VIR_DOMAIN_STARTUP_POLICY_DEFAULT:
+ goto error;
- case VIR_DOMAIN_STARTUP_POLICY_DEFAULT:
- case VIR_DOMAIN_STARTUP_POLICY_LAST:
- /* this should never happen */
- break;
+ case VIR_DOMAIN_STARTUP_POLICY_LAST:
+ /* this should never happen */
+ break;
}
- VIR_DEBUG("Dropping disk '%s' on domain '%s' (UUID
'%s') "
- "due to inaccessible source '%s'",
- disk->dst, vm->def->name, uuid, disk->src);
+ virResetLastError();
- event = virDomainEventDiskChangeNewFromObj(vm, disk->src, NULL,
disk->info.alias,
-
VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START);
- if (event)
- qemuDomainEventQueue(driver, event);
+ /* For cdrom or floppy, we only remove its source definition,
+ * so, the nextdisk need to point to next disk in next loop.
+ */
+ if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ||
+ disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
+ nextdisk++;
- VIR_FREE(disk->src);
+ if (qemuDomainCheckRemoveOptionalDisk(driver, vm, disk) < 0)
+ goto error;
}
ret = 0;
-cleanup:
- virObjectUnref(cfg);
+error:
return ret;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c412ea2..7cbcbb3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3521,17 +3521,10 @@ int qemuProcessStart(virConnectPtr conn,
if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0)
goto cleanup;
- VIR_DEBUG("Checking for CDROM and floppy presence");
if (qemuDomainCheckDiskPresence(driver, vm,
flags & VIR_QEMU_PROCESS_START_COLD) < 0)
goto cleanup;
- for (i = 0; i < vm->def->ndisks; i++) {
- if (qemuDomainDetermineDiskChain(driver, vm->def->disks[i],
- false) < 0)
- goto cleanup;
- }
-
/* Get the advisory nodeset from numad if 'placement' of
* either <vcpu> or <numatune> is 'auto'.
*/
--
1.8.1.4
6:19 p.m.
New subject: [libvirt] [PATCH v3 4/4] security: restore DAC for every disk file in disk chain
---
src/security/security_dac.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index b8d1a92..7a6f9c9 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -335,6 +335,16 @@ err:
return rc;
}
+static int
+virSecurityDACRestoreDiskSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
+ const char *path,
+ size_t depth ATTRIBUTE_UNUSED,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+ return virSecurityDACRestoreSecurityFileLabel(path);
+}
+
+
static int
virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
@@ -424,7 +434,10 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr
mgr,
}
}
- return virSecurityDACRestoreSecurityFileLabel(disk->src);
+ return virDomainDiskDefForeachPath(disk,
+ false,
+ virSecurityDACRestoreDiskSecurityFileLabel,
+ NULL);
}
--
1.8.1.4
4162
days inactive
4162
days old
4 comments
1 participants
participants (1)
-
Guannan Ren