From: Eric Blake <eblake@redhat.com>
Date: Wed, 16 Nov 2011 00:19:20 +0000 (-0700)
Subject: snapshot: refuse to generate names for non-regular backing files
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=c74b97156f9355185391856a02bbb27d98a25ba4;p=libvirt.git

snapshot: refuse to generate names for non-regular backing files

For whatever reason, the kernel allows you to create a regular
file named /dev/sdc.12345; although this file will disappear the
next time devtmpfs is remounted.  If you let libvirt generate
the name of the external snapshot for a disk image originally
using the block device /dev/sdc, then the domain will be rendered
unbootable once the qcow2 file is lost on the next devtmpfs
remount.  In this case, the user should have used 'virsh
snapshot-create --xmlfile' or 'virsh snapshot-create-as --diskspec'
to specify the name for the qcow2 file in a sane location, rather
than relying on libvirt generating a name that is most likely to
be wrong.  We can help avoid naive mistakes by enforcing that
the user provide the external name for any backing file that is
not a regular file.

* src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only
generate names if backing file exists as regular file.
Reported by MATSUDA Daiki.
---

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6b78d97e44..1cef2be6c1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12203,7 +12203,8 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
 
     qsort(&def->disks[0], def->ndisks, sizeof(def->disks[0]), disksorter);
 
-    /* Generate any default external file names.  */
+    /* Generate any default external file names, but only if the
+     * backing file is a regular file.  */
     for (i = 0; i < def->ndisks; i++) {
         virDomainSnapshotDiskDefPtr disk = &def->disks[i];
 
@@ -12211,14 +12212,24 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
             !disk->file) {
             const char *original = def->dom->disks[i]->src;
             const char *tmp;
+            struct stat sb;
 
             if (!original) {
                 virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                     _("cannot generate external backup name "
+                                     _("cannot generate external snapshot name "
                                        "for disk '%s' without source"),
                                      disk->name);
                 goto cleanup;
             }
+            if (stat(original, &sb) < 0 || !S_ISREG(sb.st_mode)) {
+                virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                     _("source for disk '%s' is not a regular "
+                                       "file; refusing to generate external "
+                                       "snapshot name"),
+                                     disk->name);
+                goto cleanup;
+            }
+
             tmp = strrchr(original, '.');
             if (!tmp || strchr(tmp, '/')) {
                 ignore_value(virAsprintf(&disk->file, "%s.%s",