Patchwork [V3,13/13] hw/sd.c: introduce "eject" property for SD card objects

login
register
mail settings
Submitter Mitsyanko Igor
Date April 27, 2012, 3:50 p.m.
Message ID <1335541859-28829-14-git-send-email-i.mitsyanko@samsung.com>
Download mbox | patch
Permalink /patch/155532/
State New
Headers show

Comments

Mitsyanko Igor - April 27, 2012, 3:50 p.m.
Boolean property "eject" could be used to query if virtual media is inserted into
SD card object, or to deattach BlockDriverState from SD card object.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
---
 hw/sd.c |   60 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 51 insertions(+), 9 deletions(-)
Paolo Bonzini - April 28, 2012, 2:27 p.m.
Il 27/04/2012 17:50, Igor Mitsyanko ha scritto:
> Boolean property "eject" could be used to query if virtual media is inserted into
> SD card object, or to deattach BlockDriverState from SD card object.

All this is already available in the BlockDriverState object; the eject
monitor command does bdrv_close and calls the change_media_cb on the
attached device.  The SD card should just attach callbacks to the BDS.

Paolo

> Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
> ---
>  hw/sd.c |   60 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 51 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/sd.c b/hw/sd.c
> index d067ffb..de57000 100644
> --- a/hw/sd.c
> +++ b/hw/sd.c
> @@ -508,6 +508,19 @@ static void sd_init_card(SDState *sd, BlockDriverState *bdrv, Error **errp)
>      }
>  }
>  
> +static void sd_deinit_card(SDState *sd)
> +{
> +    bdrv_close(sd->bdrv);
> +    sd->enable = false;
> +    bdrv_detach_dev(sd->bdrv, sd);
> +    sd_reset(sd, NULL);
> +
> +    if (sd->buf) {
> +        qemu_vfree(sd->buf);
> +        sd->buf = NULL;
> +    }
> +}
> +
>  static void sd_set_callbacks(SDState *sd, qemu_irq readonly, qemu_irq insert)
>  {
>      sd->readonly_cb = readonly;
> @@ -1828,15 +1841,7 @@ static void sd_devid_set(Object *obj, const char *value, Error **errp)
>              return;
>          }
>  
> -        bdrv_close(sd->bdrv);
> -        sd->enable = false;
> -        bdrv_detach_dev(sd->bdrv, sd);
> -        if (sd->buf) {
> -            qemu_vfree(sd->buf);
> -            sd->buf = NULL;
> -        }
> -        sd_reset(sd, NULL);
> -
> +        sd_deinit_card(sd);
>          sd_init_card(sd, bdrv, errp);
>          if (error_is_set(errp)) {
>              vmstate_unregister(NULL, &sd_vmstate, sd);
> @@ -1850,6 +1855,41 @@ static void sd_devid_set(Object *obj, const char *value, Error **errp)
>      }
>  }
>  
> +static void sd_is_ejected(Object *obj, Visitor *v, void *opaque,
> +                         const char *name, Error **errp)
> +{
> +    SDState *sd = SD_CARD(obj);
> +    bool ejected = sd->bdrv && bdrv_is_inserted(sd->bdrv) ? false : true;
> +
> +    visit_type_bool(v, &ejected, name, errp);
> +}
> +
> +static void sd_eject(Object *obj, Visitor *v, void *opaque,
> +                         const char *name, Error **errp)
> +{
> +    SDState *sd = SD_CARD(obj);
> +    bool eject;
> +
> +    visit_type_bool(v, &eject, name, errp);
> +    if (error_is_set(errp)) {
> +        return;
> +    }
> +
> +    if (eject) {
> +        if (!sd->bdrv) {
> +            return;
> +        }
> +
> +        if (bdrv_in_use(sd->bdrv)) {
> +            error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(sd->bdrv));
> +            return;
> +        }
> +
> +        vmstate_unregister(NULL, &sd_vmstate, sd);
> +        sd_deinit_card(sd);
> +    }
> +}
> +
>  static void sd_initfn(Object *obj)
>  {
>      SDState *sd = SD_CARD(obj);
> @@ -1859,6 +1899,8 @@ static void sd_initfn(Object *obj)
>              NULL, NULL, NULL);
>      object_property_add_str(OBJECT(sd), "device-id", sd_devid_get, sd_devid_set,
>              NULL);
> +    object_property_add(obj, "eject", "boolean", sd_is_ejected, sd_eject,
> +            NULL, NULL, NULL);
>  }
>  
>  static const TypeInfo sd_type_info = {
Igor Mitsyanko - April 29, 2012, 10:48 p.m.
On 28.04.2012 5:27 PM, Paolo Bonzini wrote:
> Il 27/04/2012 17:50, Igor Mitsyanko ha scritto:
>> Boolean property "eject" could be used to query if virtual media is inserted into
>> SD card object, or to deattach BlockDriverState from SD card object.
>
> All this is already available in the BlockDriverState object; the eject
> monitor command does bdrv_close and calls the change_media_cb on the
> attached device.  The SD card should just attach callbacks to the BDS.
>

The idea here was to allow for detaching BlockDriverState from SDCard 
while detached BlockDriverState object still exist in QEMU and could be 
attached to another block frontend or deallocated if it's no longer 
needed, how else "drive_del" command can work?

But I want to say, after I used this property a bit I've realized that 
its not doing what I really wanted it to. It shouldn't call 
bdrv_close(), what it should do is to call  bdrv_flush() and 
bdrv_dev_change_media_cb() and then bdrv_detach_dev(), this way there 
will be no link between SD card and BlockDriverState objects but image 
file will still be attached to BlockDriverState and could be used in 
another SD card object by setting its SDState::"drive" property 
appropriately.

> Paolo
>
>> Signed-off-by: Igor Mitsyanko<i.mitsyanko@samsung.com>
>> ---
>>   hw/sd.c |   60 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
>>   1 files changed, 51 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/sd.c b/hw/sd.c
>> index d067ffb..de57000 100644
>> --- a/hw/sd.c
>> +++ b/hw/sd.c
>> @@ -508,6 +508,19 @@ static void sd_init_card(SDState *sd, BlockDriverState *bdrv, Error **errp)
>>       }
>>   }
>>
>> +static void sd_deinit_card(SDState *sd)
>> +{
>> +    bdrv_close(sd->bdrv);
>> +    sd->enable = false;
>> +    bdrv_detach_dev(sd->bdrv, sd);
>> +    sd_reset(sd, NULL);
>> +
>> +    if (sd->buf) {
>> +        qemu_vfree(sd->buf);
>> +        sd->buf = NULL;
>> +    }
>> +}
>> +
>>   static void sd_set_callbacks(SDState *sd, qemu_irq readonly, qemu_irq insert)
>>   {
>>       sd->readonly_cb = readonly;
>> @@ -1828,15 +1841,7 @@ static void sd_devid_set(Object *obj, const char *value, Error **errp)
>>               return;
>>           }
>>
>> -        bdrv_close(sd->bdrv);
>> -        sd->enable = false;
>> -        bdrv_detach_dev(sd->bdrv, sd);
>> -        if (sd->buf) {
>> -            qemu_vfree(sd->buf);
>> -            sd->buf = NULL;
>> -        }
>> -        sd_reset(sd, NULL);
>> -
>> +        sd_deinit_card(sd);
>>           sd_init_card(sd, bdrv, errp);
>>           if (error_is_set(errp)) {
>>               vmstate_unregister(NULL,&sd_vmstate, sd);
>> @@ -1850,6 +1855,41 @@ static void sd_devid_set(Object *obj, const char *value, Error **errp)
>>       }
>>   }
>>
>> +static void sd_is_ejected(Object *obj, Visitor *v, void *opaque,
>> +                         const char *name, Error **errp)
>> +{
>> +    SDState *sd = SD_CARD(obj);
>> +    bool ejected = sd->bdrv&&  bdrv_is_inserted(sd->bdrv) ? false : true;
>> +
>> +    visit_type_bool(v,&ejected, name, errp);
>> +}
>> +
>> +static void sd_eject(Object *obj, Visitor *v, void *opaque,
>> +                         const char *name, Error **errp)
>> +{
>> +    SDState *sd = SD_CARD(obj);
>> +    bool eject;
>> +
>> +    visit_type_bool(v,&eject, name, errp);
>> +    if (error_is_set(errp)) {
>> +        return;
>> +    }
>> +
>> +    if (eject) {
>> +        if (!sd->bdrv) {
>> +            return;
>> +        }
>> +
>> +        if (bdrv_in_use(sd->bdrv)) {
>> +            error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(sd->bdrv));
>> +            return;
>> +        }
>> +
>> +        vmstate_unregister(NULL,&sd_vmstate, sd);
>> +        sd_deinit_card(sd);
>> +    }
>> +}
>> +
>>   static void sd_initfn(Object *obj)
>>   {
>>       SDState *sd = SD_CARD(obj);
>> @@ -1859,6 +1899,8 @@ static void sd_initfn(Object *obj)
>>               NULL, NULL, NULL);
>>       object_property_add_str(OBJECT(sd), "device-id", sd_devid_get, sd_devid_set,
>>               NULL);
>> +    object_property_add(obj, "eject", "boolean", sd_is_ejected, sd_eject,
>> +            NULL, NULL, NULL);
>>   }
>>
>>   static const TypeInfo sd_type_info = {
>
>
>

Patch

diff --git a/hw/sd.c b/hw/sd.c
index d067ffb..de57000 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -508,6 +508,19 @@  static void sd_init_card(SDState *sd, BlockDriverState *bdrv, Error **errp)
     }
 }
 
+static void sd_deinit_card(SDState *sd)
+{
+    bdrv_close(sd->bdrv);
+    sd->enable = false;
+    bdrv_detach_dev(sd->bdrv, sd);
+    sd_reset(sd, NULL);
+
+    if (sd->buf) {
+        qemu_vfree(sd->buf);
+        sd->buf = NULL;
+    }
+}
+
 static void sd_set_callbacks(SDState *sd, qemu_irq readonly, qemu_irq insert)
 {
     sd->readonly_cb = readonly;
@@ -1828,15 +1841,7 @@  static void sd_devid_set(Object *obj, const char *value, Error **errp)
             return;
         }
 
-        bdrv_close(sd->bdrv);
-        sd->enable = false;
-        bdrv_detach_dev(sd->bdrv, sd);
-        if (sd->buf) {
-            qemu_vfree(sd->buf);
-            sd->buf = NULL;
-        }
-        sd_reset(sd, NULL);
-
+        sd_deinit_card(sd);
         sd_init_card(sd, bdrv, errp);
         if (error_is_set(errp)) {
             vmstate_unregister(NULL, &sd_vmstate, sd);
@@ -1850,6 +1855,41 @@  static void sd_devid_set(Object *obj, const char *value, Error **errp)
     }
 }
 
+static void sd_is_ejected(Object *obj, Visitor *v, void *opaque,
+                         const char *name, Error **errp)
+{
+    SDState *sd = SD_CARD(obj);
+    bool ejected = sd->bdrv && bdrv_is_inserted(sd->bdrv) ? false : true;
+
+    visit_type_bool(v, &ejected, name, errp);
+}
+
+static void sd_eject(Object *obj, Visitor *v, void *opaque,
+                         const char *name, Error **errp)
+{
+    SDState *sd = SD_CARD(obj);
+    bool eject;
+
+    visit_type_bool(v, &eject, name, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (eject) {
+        if (!sd->bdrv) {
+            return;
+        }
+
+        if (bdrv_in_use(sd->bdrv)) {
+            error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(sd->bdrv));
+            return;
+        }
+
+        vmstate_unregister(NULL, &sd_vmstate, sd);
+        sd_deinit_card(sd);
+    }
+}
+
 static void sd_initfn(Object *obj)
 {
     SDState *sd = SD_CARD(obj);
@@ -1859,6 +1899,8 @@  static void sd_initfn(Object *obj)
             NULL, NULL, NULL);
     object_property_add_str(OBJECT(sd), "device-id", sd_devid_get, sd_devid_set,
             NULL);
+    object_property_add(obj, "eject", "boolean", sd_is_ejected, sd_eject,
+            NULL, NULL, NULL);
 }
 
 static const TypeInfo sd_type_info = {