diff mbox

[2/4] usb: add attached property

Message ID 1453804885-15544-3-git-send-email-kraxel@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gerd Hoffmann Jan. 26, 2016, 10:41 a.m. UTC
USB devices in attached state are visible to the guest.  This patch adds
a QOM property for this.  Write access is opt-in per device.  Some
devices manage attached state automatically (usb-host, usb-serial), so
we can't enable write access universally but have to do it on a case by
case base.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/bus.c     | 42 ++++++++++++++++++++++++++++++++++++++++++
 include/hw/usb.h |  1 +
 2 files changed, 43 insertions(+)

Comments

Markus Armbruster Feb. 2, 2016, 1:33 p.m. UTC | #1
Suggest to say

    usb: Add QOM property "attached"

The quotes make more obvious that "attached" is a property name, not an
adjective tacked to property.

Gerd Hoffmann <kraxel@redhat.com> writes:

> USB devices in attached state are visible to the guest.

If I read the code correctly:

* ->attached is true between usb_device_attach() and usb_device_detach()

* Attach and detach is automatic on realize and unrealize, but a device
  can choose to suppress attach on realize, by unsetting ->auto_attach,
  and manage attach/detach itself.

* A detached device is wired up normally in QOM, but the USB core treats
  it as not plugged into its USB port.

Correct?

>                                                          This patch adds
> a QOM property for this.  Write access is opt-in per device.  Some
> devices manage attached state automatically (usb-host, usb-serial), so

Full list of devices unsetting ->auto_attach: dev-serial.c dev-storage.c
host-libusb.c redirect.c.

dev-storage.c does it only for encrypted media without a key, but that
should go away with Dan's "Support LUKS encryption in block devices", so
I guess neglecting to mention it here is okay.

Suggest to add usb-redir to your list.

> we can't enable write access universally but have to do it on a case by
> case base.

Suggest to add: So far, no device opts in.

>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Patch looks good.
Gerd Hoffmann Feb. 2, 2016, 2:25 p.m. UTC | #2
Hi,

> > USB devices in attached state are visible to the guest.
> 
> If I read the code correctly:
> 
> * ->attached is true between usb_device_attach() and usb_device_detach()
> 
> * Attach and detach is automatic on realize and unrealize, but a device
>   can choose to suppress attach on realize, by unsetting ->auto_attach,
>   and manage attach/detach itself.
> 
> * A detached device is wired up normally in QOM, but the USB core treats
>   it as not plugged into its USB port.
> 
> Correct?

Yes.

cheers,
  Gerd
Markus Armbruster Feb. 2, 2016, 3:37 p.m. UTC | #3
Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
>> > USB devices in attached state are visible to the guest.
>> 
>> If I read the code correctly:
>> 
>> * ->attached is true between usb_device_attach() and usb_device_detach()
>> 
>> * Attach and detach is automatic on realize and unrealize, but a device
>>   can choose to suppress attach on realize, by unsetting ->auto_attach,
>>   and manage attach/detach itself.
>> 
>> * A detached device is wired up normally in QOM, but the USB core treats
>>   it as not plugged into its USB port.
>> 
>> Correct?
>
> Yes.

Preferrably with the commit message improved along the line of my
suggestions:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff mbox

Patch

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index dd28041..17e0479 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -733,6 +733,47 @@  USBDevice *usbdevice_create(const char *cmdline)
     return dev;
 }
 
+static bool usb_get_attached(Object *obj, Error **errp)
+{
+    USBDevice *dev = USB_DEVICE(obj);
+
+    return dev->attached;
+}
+
+static void usb_set_attached(Object *obj, bool value, Error **errp)
+{
+    USBDevice *dev = USB_DEVICE(obj);
+    Error *err = NULL;
+
+    if (dev->attached == value)
+        return;
+
+    if (value) {
+        usb_device_attach(dev, &err);
+        if (err) {
+            error_propagate(errp, err);
+        }
+    } else {
+        usb_device_detach(dev);
+    }
+}
+
+static void usb_device_instance_init(Object *obj)
+{
+    USBDevice *dev = USB_DEVICE(obj);
+    USBDeviceClass *klass = USB_DEVICE_GET_CLASS(dev);
+
+    if (klass->attached_settable) {
+        object_property_add_bool(obj, "attached",
+                                 usb_get_attached, usb_set_attached,
+                                 NULL);
+    } else {
+        object_property_add_bool(obj, "attached",
+                                 usb_get_attached, NULL,
+                                 NULL);
+    }
+}
+
 static void usb_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
@@ -746,6 +787,7 @@  static const TypeInfo usb_device_type_info = {
     .name = TYPE_USB_DEVICE,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(USBDevice),
+    .instance_init = usb_device_instance_init,
     .abstract = true,
     .class_size = sizeof(USBDeviceClass),
     .class_init = usb_device_class_init,
diff --git a/include/hw/usb.h b/include/hw/usb.h
index f8432f9..6eaa19c 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -346,6 +346,7 @@  typedef struct USBDeviceClass {
 
     const char *product_desc;
     const USBDesc *usb_desc;
+    bool attached_settable;
 } USBDeviceClass;
 
 typedef struct USBPortOps {