Re: [libvirt] [PATCH] Support QEMU's virtual FAT block device driver

On Tue, Nov 17, 2009 at 01:20:15PM +0000, Daniel P. Berrange wrote: > Introduce a new type="dir" mode for <disks> that allows use of > QEMU's virtual FAT block device driver. eg > > <disk type='dir' device='floppy'> > <source dir='/tmp/test'/> > <target dev='fda' bus='fdc'/> > <readonly/> > </disk> > > gets turned into > > -drive file=fat:floppy:/tmp/test,if=floppy,index=0 > > Only read-only disks are supported with virtual FAT mode > > * src/conf/domain_conf.c, src/conf/domain_conf.h: Add type="dir" > * docs/schemas/domain.rng: Document new disk type > * src/xen/xend_internal.c, src/xen/xm_internal.c: Raise error for > unsupported disk types > * tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-empty.args: Fix > empty disk file handling > * tests/qemuxml2argvdata/qemuxml2argv-disk-drive-fat.args, > tests/qemuxml2argvdata/qemuxml2argv-disk-drive-fat.xml, > tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.args, > tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.xml > tests/qemuxml2argvtest.c: Test QEMU vitual FAT driver > * src/qemu/qemu_conf.c: Support generating fat:/some/dir type > disk args > * src/security/security_selinux.c: Temporarily skip labelling > of directory based disks > --- > docs/schemas/domain.rng | 16 ++++++ > src/conf/domain_conf.c | 36 ++++++++++++-- > src/conf/domain_conf.h | 1 + > src/qemu/qemu_conf.c | 49 ++++++++++++++++++- > src/security/security_selinux.c | 3 + > src/xen/xend_internal.c | 7 ++- > src/xen/xm_internal.c | 16 +++++- > .../qemuxml2argv-disk-cdrom-empty.args | 2 +- > .../qemuxml2argv-disk-drive-fat.args | 1 + > .../qemuxml2argv-disk-drive-fat.xml | 24 ++++++++++ > .../qemuxml2argv-floppy-drive-fat.args | 1 + > .../qemuxml2argv-floppy-drive-fat.xml | 24 ++++++++++ > tests/qemuxml2argvtest.c | 4 ++ > tests/qemuxml2xmltest.c | 2 + > 14 files changed, 173 insertions(+), 13 deletions(-) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-fat.args > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-fat.xml > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.args > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.xml > > diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng > index 1bf44fd..7a3ef97 100644 > --- a/docs/schemas/domain.rng > +++ b/docs/schemas/domain.rng > @@ -434,6 +434,22 @@ > <ref name="diskspec"/> > </interleave> > </group> > + <group> > + <attribute name="type"> > + <value>dir</value> > + </attribute> > + <interleave> > + <optional> > + <element name="source"> > + <attribute name="dir"> > + <ref name="absFilePath"/> > + </attribute> > + <empty/> > + </element> > + </optional> > + <ref name="diskspec"/> > + </interleave> > + </group> > <ref name="diskspec"/> > </choice> > </element> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 0a7eef7..42820a7 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -90,7 +90,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, > > VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, > "block", > - "file") > + "file", > + "dir") > > VIR_ENUM_IMPL(virDomainDiskDevice, VIR_DOMAIN_DISK_DEVICE_LAST, > "disk", > @@ -777,10 +778,22 @@ virDomainDiskDefParseXML(virConnectPtr conn, > if ((source == NULL) && > (xmlStrEqual(cur->name, BAD_CAST "source"))) { > > - if (def->type == VIR_DOMAIN_DISK_TYPE_FILE) > + switch (def->type) { > + case VIR_DOMAIN_DISK_TYPE_FILE: > source = virXMLPropString(cur, "file"); > - else > + break; > + case VIR_DOMAIN_DISK_TYPE_BLOCK: > source = virXMLPropString(cur, "dev"); > + break; > + case VIR_DOMAIN_DISK_TYPE_DIR: > + source = virXMLPropString(cur, "dir"); > + break; > + default: > + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, > + _("unexpected disk type %s"), > + virDomainDiskTypeToString(def->type)); > + goto error; > + } > > /* People sometimes pass a bogus '' source path > when they mean to omit the source element > @@ -3951,12 +3964,25 @@ virDomainDiskDefFormat(virConnectPtr conn, > } > > if (def->src) { > - if (def->type == VIR_DOMAIN_DISK_TYPE_FILE) > + switch (def->type) { > + case VIR_DOMAIN_DISK_TYPE_FILE: > virBufferEscapeString(buf, " <source file='%s'/>\n", > def->src); > - else > + break; > + case VIR_DOMAIN_DISK_TYPE_BLOCK: > virBufferEscapeString(buf, " <source dev='%s'/>\n", > def->src); > + break; > + case VIR_DOMAIN_DISK_TYPE_DIR: > + virBufferEscapeString(buf, " <source dir='%s'/>\n", > + def->src); > + break; > + default: > + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, > + _("unexpected disk type %s"), > + virDomainDiskTypeToString(def->type)); > + return -1; > + } > } > > virBufferVSprintf(buf, " <target dev='%s' bus='%s'/>\n", > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 1fdb4fa..6201463 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -67,6 +67,7 @@ enum virDomainVirtType { > enum virDomainDiskType { > VIR_DOMAIN_DISK_TYPE_BLOCK, > VIR_DOMAIN_DISK_TYPE_FILE, > + VIR_DOMAIN_DISK_TYPE_DIR, > > VIR_DOMAIN_DISK_TYPE_LAST > }; > diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c > index c807688..2d72b4b 100644 > --- a/src/qemu/qemu_conf.c > +++ b/src/qemu/qemu_conf.c > @@ -1980,8 +1980,30 @@ int qemudBuildCommandLine(virConnectPtr conn, > break; > } > > - virBufferVSprintf(&opt, "file=%s", disk->src ? disk->src : ""); > - virBufferVSprintf(&opt, ",if=%s", bus); > + if (disk->src) { > + if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { > + /* QEMU only supports magic FAT format for now */ > + if (disk->driverType && > + STRNEQ(disk->driverType, "fat")) { > + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, > + _("unsupported disk driver type for '%s'"), > + disk->driverType); > + goto error; > + } > + if (!disk->readonly) { > + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", > + _("cannot create virtual FAT disks in read-write mode")); > + goto error; > + } > + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) > + virBufferVSprintf(&opt, "file=fat:floppy:%s,", disk->src); > + else > + virBufferVSprintf(&opt, "file=fat:%s,", disk->src); > + } else { > + virBufferVSprintf(&opt, "file=%s,", disk->src); > + } > + } > + virBufferVSprintf(&opt, "if=%s", bus); > if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) > virBufferAddLit(&opt, ",media=cdrom"); > virBufferVSprintf(&opt, ",index=%d", idx); > @@ -1989,6 +2011,7 @@ int qemudBuildCommandLine(virConnectPtr conn, > disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) > virBufferAddLit(&opt, ",boot=on"); > if (disk->driverType && > + disk->type != VIR_DOMAIN_DISK_TYPE_DIR && > qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_FORMAT) > virBufferVSprintf(&opt, ",format=%s", disk->driverType); > if (disk->serial && > @@ -2057,7 +2080,27 @@ int qemudBuildCommandLine(virConnectPtr conn, > } > } > > - snprintf(file, PATH_MAX, "%s", disk->src); > + if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { > + /* QEMU only supports magic FAT format for now */ > + if (disk->driverType && > + STRNEQ(disk->driverType, "fat")) { > + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, > + _("unsupported disk driver type for '%s'"), > + disk->driverType); > + goto error; > + } > + if (!disk->readonly) { > + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", > + _("cannot create virtual FAT disks in read-write mode")); > + goto error; > + } > + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) > + snprintf(file, PATH_MAX, "fat:floppy:%s", disk->src); > + else > + snprintf(file, PATH_MAX, "fat:%s", disk->src); > + } else { > + snprintf(file, PATH_MAX, "%s", disk->src); > + } > > ADD_ARG_LIT(dev); > ADD_ARG_LIT(file); > diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c > index bd838e6..255ba53 100644 > --- a/src/security/security_selinux.c > +++ b/src/security/security_selinux.c > @@ -687,6 +687,9 @@ SELinuxSetSecurityLabel(virConnectPtr conn, > > if (secdef->imagelabel) { > for (i = 0 ; i < vm->def->ndisks ; i++) { > + /* XXX fixme - we need to recursively label the entriy tree :-( */ > + if (vm->def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_DIR) > + continue; > if (SELinuxSetSecurityImageLabel(conn, vm, vm->def->disks[i]) < 0) > return -1; > } > diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c > index d61e9e6..e370eb8 100644 > --- a/src/xen/xend_internal.c > +++ b/src/xen/xend_internal.c > @@ -5375,11 +5375,16 @@ xenDaemonFormatSxprDisk(virConnectPtr conn ATTRIBUTE_UNUSED, > } else { > if (def->type == VIR_DOMAIN_DISK_TYPE_FILE) { > virBufferVSprintf(buf, "(uname 'file:%s')", def->src); > - } else { > + } else if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { > if (def->src[0] == '/') > virBufferVSprintf(buf, "(uname 'phy:%s')", def->src); > else > virBufferVSprintf(buf, "(uname 'phy:/dev/%s')", def->src); > + } else { > + virXendError(conn, VIR_ERR_CONFIG_UNSUPPORTED, > + _("unsupported disk type %s"), > + virDomainDiskTypeToString(def->type)); > + return -1; > } > } > } > diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c > index f833ce7..31cff12 100644 > --- a/src/xen/xm_internal.c > +++ b/src/xen/xm_internal.c > @@ -1973,9 +1973,19 @@ static int xenXMDomainConfigFormatDisk(virConnectPtr conn, > if (STREQ(disk->driverName, "tap")) > virBufferVSprintf(&buf, "%s:", disk->driverType ? disk->driverType : "aio"); > } else { > - virBufferVSprintf(&buf, "%s:", > - disk->type == VIR_DOMAIN_DISK_TYPE_FILE ? > - "file" : "phy"); > + switch (disk->type) { > + case VIR_DOMAIN_DISK_TYPE_FILE: > + virBufferAddLit(&buf, "file:"); > + break; > + case VIR_DOMAIN_DISK_TYPE_BLOCK: > + virBufferAddLit(&buf, "phy:"); > + break; > + default: > + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, > + _("unsupported disk type %s"), > + virDomainDiskTypeToString(disk->type)); > + goto cleanup; > + } > } > virBufferVSprintf(&buf, "%s", disk->src); > } > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-empty.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-empty.args > index 1ef2602..1dd90d0 100644 > --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-empty.args > +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-empty.args > @@ -1 +1 @@ > -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,index=0 -drive file=,if=ide,media=cdrom,index=2 -net none -serial none -parallel none -usb > +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,index=0 -drive if=ide,media=cdrom,index=2 -net none -serial none -parallel none -usb > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-fat.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-fat.args > new file mode 100644 > index 0000000..da1163b > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-fat.args > @@ -0,0 +1 @@ > +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=fat:/var/somefiles,if=ide,index=0,boot=on -net none -serial none -parallel none -usb > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-fat.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-fat.xml > new file mode 100644 > index 0000000..818ca93 > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-fat.xml > @@ -0,0 +1,24 @@ > +<domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory>219200</memory> > + <currentMemory>219200</currentMemory> > + <vcpu>1</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='hd'/> > + </os> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu</emulator> > + <disk type='dir' device='disk'> > + <driver name='qemu' type='fat'/> > + <source dir='/var/somefiles'/> > + <target dev='hda' bus='ide'/> > + <readonly/> > + </disk> > + </devices> > +</domain> > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.args b/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.args > new file mode 100644 > index 0000000..4b4e3f4 > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.args > @@ -0,0 +1 @@ > +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot a -drive file=fat:floppy:/var/somefiles,if=floppy,index=0 -net none -serial none -parallel none -usb > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.xml b/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.xml > new file mode 100644 > index 0000000..9e32b68 > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.xml > @@ -0,0 +1,24 @@ > +<domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory>219200</memory> > + <currentMemory>219200</currentMemory> > + <vcpu>1</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='fd'/> > + </os> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu</emulator> > + <disk type='dir' device='floppy'> > + <driver name='qemu' type='fat'/> > + <source dir='/var/somefiles'/> > + <target dev='fda' bus='fdc'/> > + <readonly/> > + </disk> > + </devices> > +</domain> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index c948379..677c5b4 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -211,6 +211,10 @@ mymain(int argc, char **argv) > QEMUD_CMD_FLAG_DRIVE_BOOT); > DO_TEST("disk-drive-boot-cdrom", QEMUD_CMD_FLAG_DRIVE | > QEMUD_CMD_FLAG_DRIVE_BOOT); > + DO_TEST("floppy-drive-fat", QEMUD_CMD_FLAG_DRIVE | > + QEMUD_CMD_FLAG_DRIVE_BOOT | QEMUD_CMD_FLAG_DRIVE_FORMAT); > + DO_TEST("disk-drive-fat", QEMUD_CMD_FLAG_DRIVE | > + QEMUD_CMD_FLAG_DRIVE_BOOT | QEMUD_CMD_FLAG_DRIVE_FORMAT); > DO_TEST("disk-drive-fmt-qcow", QEMUD_CMD_FLAG_DRIVE | > QEMUD_CMD_FLAG_DRIVE_BOOT | QEMUD_CMD_FLAG_DRIVE_FORMAT); > DO_TEST("disk-drive-shared", QEMUD_CMD_FLAG_DRIVE | > diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c > index 25ef2ce..793900c 100644 > --- a/tests/qemuxml2xmltest.c > +++ b/tests/qemuxml2xmltest.c > @@ -98,6 +98,8 @@ mymain(int argc, char **argv) > DO_TEST("disk-many"); > DO_TEST("disk-xenvbd"); > DO_TEST("disk-usb"); > + DO_TEST("floppy-drive-fat"); > + DO_TEST("disk-drive-fat"); > DO_TEST("disk-drive-fmt-qcow"); > DO_TEST("disk-drive-cache-v1-wt"); > DO_TEST("disk-drive-cache-v1-wb"); ACK, that's funky, and I guess this will stay a qemu only feature. One thing I wonder about the semantic is what happen if the underlying files get modified while QEmu tries to access that 'floppy'. I assume "domain can crash an burn" is just fine