[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Tue, Nov 17, 2009 at 04:08:08PM +0100, Daniel Veillard wrote:
> 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

Yes, QEMU has this to say

  "What you should never do:

    * use non-ASCII filenames ;
    * use "-snapshot" together with ":rw:" ;
    * expect it to work when loadvm'ing ;
    * write to the FAT directory on the host system while 
      accessing it with the guest system. "

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]