Re: [Qemu-devel] [PATCH V2 10/10] hw/sd.c: introduce SD card "image" property and allow SD hot-insert

[Igor Mitsyanko]

On 04/05/2012 09:02 PM, Paolo Bonzini wrote:
> Il 05/04/2012 17:48, Igor Mitsyanko ha scritto:
> > New SD card "image" property can be used to:
> > - change image file attached to virtual SD card
> > - hot-insert new image file into newly initialized BlockDriverState (this was 
> > not
> > possible before).
> >
> > Example usage:
> > ./qom-set /machine/milkymist/milkymist-memcard/card.image /home/me/mynewcard.img
> > this will attach image file /home/me/mynewcard.img to virtual SD card connected 
> > to
> > milkymist-memcard host controller device. If virtual card was already attached 
> > to
> > some other image file, eject event is triggered before attaching new file.
> >
> > Signed-off-by: Igor Mitsyanko<address@hidden>
> > ---
> >   hw/sd.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
> >   1 files changed, 44 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/sd.c b/hw/sd.c
> > index 8ffaa17..3e75405 100644
> > --- a/hw/sd.c
> > +++ b/hw/sd.c
> > @@ -1816,6 +1816,48 @@ static void sd_set_spimode(Object *obj, Visitor *v, void 
> > *opaque,
> >       }
> >   }
> >
> > +static void sd_set_image_path(Object *obj, Visitor *v, void *opaque,
> > +                         const char *name, Error **errp)
> > +{
> > +    SDState *sd = SD_CARD(obj);
> > +    char *new_image;
> > +
> > +    visit_type_str(v,&new_image, "file", errp);
> Please use the name argument instead of "file".
OK
> > +
> > +    if (error_is_set(errp)) {
> > +        return;
> > +    }
> > +
> > +    if (sd->bdrv) {
> > +        qmp_change_blockdev(bdrv_get_device_name(sd->bdrv), new_image,
> > +                false, NULL, errp);
> > +    } else {
> > +        DriveInfo *di;
> > +        QemuOpts *opts = drive_add(IF_SD, sd->if_idx, new_image, "");
> I think this should simply take a drive name and pass it to
> bdrv_get_device_name.
Did you mean bdrv_find()? I can't use it here, BlockDriverState doesn't 
exist yet, I need to create
it first with drive_add.

>   The drive_add/drive_init should be done
> separately with the drive_add monitor command, like
>
> drive_add 0 file=foo.img,if=none,id=bar
So you're saying we need to use two commands to insert a new virtual SD 
card:
 first we need to create a BlockDriverState with specified device_name
 and then with second command we associate SD card with previously created
BlockDriverState?
I kind of like another approach, when we can just set filename as SD 
card's property and
BlockDriverState will be automatically created and connected to this SD 
card's state. And when we
want to eject card from virtual slot, we set SDCard::eject property to 
true and then associated
BlockDriverState is dereferenced and freed automatically.
> With the upcoming support for static properties in objects that are not
> devices, you can just add a drive property to the SD class.
>
> There is a problem here however.  QOM commands are HMP only, and
> drive_add is QMP only.  I think that blocking drive_add in QMP is
> perfect being the enemy of good.  Alternatively, however, adding the QOM
> property commands to HMP would also make sense.
I don't understand, is it some kind of conceptual restriction? We just 
call visit_type_str() here, it
could be called by object_property_set_str(), or from QMP socket, or 
from monitor for all we
care, what's the difference?

--
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: address@hidden