qemu: Support setting the 'removable' flag for USB disks

Add an attribute named 'removable' to the 'target' element of disks,
which controls the removable flag. For instance, on a Linux guest it
controls the value of /sys/block/$dev/removable. This option is only
valid for USB disks (i.e. bus='usb'), and its default value is 'off',
which is the same behaviour as before.

To achieve this, 'removable=on' (or 'off') is appended to the '-device
usb-storage' parameter sent to qemu when adding a USB disk via
'-disk'. A capability flag QEMU_CAPS_USB_STORAGE_REMOVABLE was added
to keep track if this option is supported by the qemu version used.

Bug: https://bugzilla.redhat.com/show_bug.cgi?id=922495
Signed-off-by: Peter Krempa <pkrempa@redhat.com>

docs/formatdomain.html.in

@@ -1792,9 +1792,13 @@
         removable disks (i.e. CDROM or Floppy disk), the value can be either
         "open" or "closed", defaults to "closed". NB, the value of
         <code>tray</code> could be updated while the domain is running.
-        <span class="since">Since 0.0.3; <code>bus</code> attribute since 0.4.3;
+        The optional attribute <code>removable</code> sets the
+        removable flag for USB disks, and its value can be either "on"
+        or "off", defaulting to "off". <span class="since">Since
+        0.0.3; <code>bus</code> attribute since 0.4.3;
         <code>tray</code> attribute since 0.9.11; "usb" attribute value since
-        after 0.4.4; "sata" attribute value since 0.9.7</span>
+        after 0.4.4; "sata" attribute value since 0.9.7; "removable" attribute
+        value since 1.1.3</span>
       </dd>
       <dt><code>iotune</code></dt>
       <dd>The optional <code>iotune</code> element provides the

docs/schemas/domaincommon.rng

@@ -1287,6 +1287,14 @@
           </choice>
         </attribute>
       </optional>
+      <optional>
+        <attribute name="removable">
+          <choice>
+            <value>on</value>
+            <value>off</value>
+          </choice>
+        </attribute>
+      </optional>
     </element>
   </define>
   <define name="geometry">

src/conf/domain_conf.c

@@ -4758,6 +4758,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
     char *authUUID = NULL;
     char *usageType = NULL;
     char *tray = NULL;
+    char *removable = NULL;
     char *logical_block_size = NULL;
     char *physical_block_size = NULL;
     char *wwn = NULL;
@@ -4914,6 +4915,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                 target = virXMLPropString(cur, "dev");
                 bus = virXMLPropString(cur, "bus");
                 tray = virXMLPropString(cur, "tray");
+                removable = virXMLPropString(cur, "removable");
 
                 /* HACK: Work around for compat with Xen
                  * driver in previous libvirt releases */
@@ -5348,6 +5350,24 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
             def->tray_status = VIR_DOMAIN_DISK_TRAY_CLOSED;
     }
 
+    if (removable) {
+        if ((def->removable = virDomainFeatureStateTypeFromString(removable)) < 0) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("unknown disk removable status '%s'"), removable);
+            goto error;
+        }
+
+        if (def->bus != VIR_DOMAIN_DISK_BUS_USB) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("removable is only valid for usb disks"));
+            goto error;
+        }
+    } else {
+        if (def->bus == VIR_DOMAIN_DISK_BUS_USB) {
+            def->removable = VIR_DOMAIN_FEATURE_STATE_DEFAULT;
+        }
+    }
+
     if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY &&
         def->bus != VIR_DOMAIN_DISK_BUS_FDC) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -5553,6 +5573,7 @@ cleanup:
     VIR_FREE(target);
     VIR_FREE(source);
     VIR_FREE(tray);
+    VIR_FREE(removable);
     VIR_FREE(trans);
     while (nhosts > 0) {
         virDomainDiskHostDefFree(&hosts[nhosts - 1]);
@@ -14410,9 +14431,13 @@ virDomainDiskDefFormat(virBufferPtr buf,
     if ((def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
          def->device == VIR_DOMAIN_DISK_DEVICE_CDROM) &&
         def->tray_status != VIR_DOMAIN_DISK_TRAY_CLOSED)
-        virBufferAsprintf(buf, " tray='%s'/>\n",
+        virBufferAsprintf(buf, " tray='%s'",
                           virDomainDiskTrayTypeToString(def->tray_status));
-    else
+    if (def->bus == VIR_DOMAIN_DISK_BUS_USB &&
+        def->removable != VIR_DOMAIN_FEATURE_STATE_DEFAULT) {
+        virBufferAsprintf(buf, " removable='%s'",
+                          virDomainFeatureStateTypeToString(def->removable));
+    }
     virBufferAddLit(buf, "/>\n");
 
     /*disk I/O throttling*/

src/conf/domain_conf.h

@@ -693,6 +693,7 @@ struct _virDomainDiskDef {
     char *src;
     char *dst;
     int tray_status;
+    int removable;
     int protocol;
     size_t nhosts;
     virDomainDiskHostDefPtr hosts;

src/qemu/qemu_capabilities.c

@@ -239,6 +239,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
               "q35-pci-hole64-size",
 
               "usb-storage", /* 155 */
+              "usb-storage.removable",
     );
 
 struct _virQEMUCaps {
@@ -1449,6 +1450,10 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQ35PciHost[] = {
     { "pci-hole64-size", QEMU_CAPS_Q35_PCI_HOLE64_SIZE },
 };
 
+static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsUsbStorage[] = {
+    { "removable", QEMU_CAPS_USB_STORAGE_REMOVABLE },
+};
+
 struct virQEMUCapsObjectTypeProps {
     const char *type;
     struct virQEMUCapsStringFlags *props;
@@ -1490,6 +1495,8 @@ static struct virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = {
       ARRAY_CARDINALITY(virQEMUCapsObjectPropsI440FXPciHost) },
     { "q35-pcihost", virQEMUCapsObjectPropsQ35PciHost,
       ARRAY_CARDINALITY(virQEMUCapsObjectPropsQ35PciHost) },
+    { "usb-storage", virQEMUCapsObjectPropsUsbStorage,
+      ARRAY_CARDINALITY(virQEMUCapsObjectPropsUsbStorage) },
 };
 
 
@@ -1680,6 +1687,7 @@ virQEMUCapsExtractDeviceStr(const char *qemu,
                          "-device", "ide-drive,?",
                          "-device", "usb-host,?",
                          "-device", "scsi-generic,?",
+                         "-device", "usb-storage,?",
                          NULL);
     /* qemu -help goes to stdout, but qemu -device ? goes to stderr.  */
     virCommandSetErrorBuffer(cmd, &output);

src/qemu/qemu_capabilities.h

@@ -194,6 +194,7 @@ enum virQEMUCapsFlags {
     QEMU_CAPS_I440FX_PCI_HOLE64_SIZE = 153, /* i440FX-pcihost.pci-hole64-size */
     QEMU_CAPS_Q35_PCI_HOLE64_SIZE = 154, /* q35-pcihost.pci-hole64-size */
     QEMU_CAPS_DEVICE_USB_STORAGE = 155, /* -device usb-storage */
+    QEMU_CAPS_USB_STORAGE_REMOVABLE = 156, /* usb-storage.removable */
 
     QEMU_CAPS_LAST,                   /* this must always be the last item */
 };

src/qemu/qemu_command.c

@@ -4394,6 +4394,22 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
     if (disk->product)
         virBufferAsprintf(&opt, ",product=%s", disk->product);
 
+    if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
+        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_STORAGE_REMOVABLE)) {
+            if (disk->removable == VIR_DOMAIN_FEATURE_STATE_ON)
+                virBufferAddLit(&opt, ",removable=on");
+            else
+                virBufferAddLit(&opt, ",removable=off");
+        } else {
+            if (disk->removable != VIR_DOMAIN_FEATURE_STATE_DEFAULT) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("This QEMU doesn't support setting the "
+                                 "removable flag of USB storage devices"));
+                goto error;
+            }
+        }
+    }
+
     if (virBufferError(&opt)) {
         virReportOOMError();
         goto error;
@@ -11350,6 +11366,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
                     disk->type = VIR_DOMAIN_DISK_TYPE_FILE;
                 disk->device = VIR_DOMAIN_DISK_DEVICE_DISK;
                 disk->bus = VIR_DOMAIN_DISK_BUS_USB;
+                disk->removable = VIR_DOMAIN_FEATURE_STATE_DEFAULT;
                 if (VIR_STRDUP(disk->dst, "sda") < 0)
                     goto error;
                 if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0)

tests/qemuhelpdata/qemu-1.2.0-device

@@ -213,3 +213,14 @@ scsi-generic.bootindex=int32
 scsi-generic.channel=uint32
 scsi-generic.scsi-id=uint32
 scsi-generic.lun=uint32
+usb-storage.drive=drive
+usb-storage.logical_block_size=blocksize
+usb-storage.physical_block_size=blocksize
+usb-storage.min_io_size=uint16
+usb-storage.opt_io_size=uint32
+usb-storage.bootindex=int32
+usb-storage.discard_granularity=uint32
+usb-storage.serial=string
+usb-storage.removable=on/off
+usb-storage.port=string
+usb-storage.full-path=on/off

tests/qemuhelpdata/qemu-kvm-1.2.0-device

@@ -225,3 +225,14 @@ scsi-generic.bootindex=int32
 scsi-generic.channel=uint32
 scsi-generic.scsi-id=uint32
 scsi-generic.lun=uint32
+usb-storage.drive=drive
+usb-storage.logical_block_size=blocksize
+usb-storage.physical_block_size=blocksize
+usb-storage.min_io_size=uint16
+usb-storage.opt_io_size=uint32
+usb-storage.bootindex=int32
+usb-storage.discard_granularity=uint32
+usb-storage.serial=string
+usb-storage.removable=on/off
+usb-storage.port=string
+usb-storage.full-path=on/off

tests/qemuhelptest.c

@@ -942,7 +942,8 @@ mymain(void)
             QEMU_CAPS_DEVICE_SCSI_GENERIC,
             QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX,
             QEMU_CAPS_VNC_SHARE_POLICY,
-            QEMU_CAPS_DEVICE_USB_STORAGE);
+            QEMU_CAPS_DEVICE_USB_STORAGE,
+            QEMU_CAPS_USB_STORAGE_REMOVABLE);
     DO_TEST("qemu-kvm-1.2.0", 1002000, 1, 0,
             QEMU_CAPS_VNC_COLON,
             QEMU_CAPS_NO_REBOOT,
@@ -1054,7 +1055,8 @@ mymain(void)
             QEMU_CAPS_DEVICE_SCSI_GENERIC,
             QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX,
             QEMU_CAPS_VNC_SHARE_POLICY,
-            QEMU_CAPS_DEVICE_USB_STORAGE);
+            QEMU_CAPS_DEVICE_USB_STORAGE,
+            QEMU_CAPS_USB_STORAGE_REMOVABLE);
 
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }

tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args

@@ -0,0 +1,8 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
+pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
+unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \
+file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0 -device ide-drive,\
+bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive file=/tmp/usbdisk.img,\
+if=none,id=drive-usb-disk0 -device usb-storage,drive=drive-usb-disk0,\
+id=usb-disk0,removable=on -device virtio-balloon-pci,id=balloon0,bus=pci.0,\
+addr=0x3

tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.xml

@@ -0,0 +1,27 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>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='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <source file='/tmp/usbdisk.img'/>
+      <target dev='sda' bus='usb' removable='on'/>
+    </disk>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>

tests/qemuxml2argvtest.c

@@ -546,6 +546,9 @@ mymain(void)
     DO_TEST("disk-usb-device",
             QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_USB_STORAGE,
             QEMU_CAPS_NODEFCONFIG);
+    DO_TEST("disk-usb-device-removable",
+            QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_USB_STORAGE,
+            QEMU_CAPS_USB_STORAGE_REMOVABLE, QEMU_CAPS_NODEFCONFIG);
     DO_TEST("disk-scsi-device",
             QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
             QEMU_CAPS_SCSI_LSI);