[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] gtk: add devices menu to allow changing rem
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices |
Date: |
Thu, 2 May 2013 10:49:26 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 26.04.2013 um 21:43 hat Anthony Liguori geschrieben:
> To generate this menu, we first walk the composition tree to
> find any device with a 'drive' property. We then record these
> devices and the BlockDriverState that they are associated with.
>
> Then we use query-block to get the BDS state for each of the
> discovered devices.
>
> This code doesn't handle hot-plug yet but it should deal nicely
> with someone using the human monitor.
>
> Signed-off-by: Anthony Liguori <address@hidden>
I haven't checked what causes it, but with this patch applied I get a
screenful of GTK error messages when I exit qemu with Alt-F4.
> +static void gd_block_device_menu_update(BlockDeviceMenu *bdm, BlockInfo
> *info)
> +{
> + bool value;
> + const char *label = _("<No media>");
> +
> + value = info->has_inserted && !info->locked;
Shouldn't the actual value of info->inserted play a role as well?
> + gtk_widget_set_sensitive(bdm->eject, value);
> +
> + value = !info->locked;
> + gtk_widget_set_sensitive(bdm->change, value);
> +
> + if (info->has_inserted) {
> + label = info->inserted->file;
> + if (strlen(label) > 32) {
> + char *new_label;
> +
> + new_label = strrchr(label, '/');
> + if (new_label) {
> + label = new_label + 1;
> + }
> + }
> + }
> +
> + gtk_menu_item_set_label(GTK_MENU_ITEM(bdm->change), label);
> +}
> +static void gd_enum_disk(const char *path, const char *proptype, void
> *opaque)
> +{
> + GtkDisplayState *s = opaque;
> + Object *obj;
> + char *block_id;
> +
> + obj = object_resolve_path(path, NULL);
> + g_assert(obj != NULL);
> +
> + block_id = object_property_get_str(obj, proptype, NULL);
> + if (strcmp(block_id, "") != 0) {
> + BlockDeviceMenu *bdm;
> + DiskType disk_type;
> + char *type;
> + char *desc = NULL;
> +
> + type = object_property_get_str(obj, "type", NULL);
> +
> + if (strcmp(type, "ide-cd") == 0 || strcmp(type, "ide-hd") == 0) {
> + desc = object_property_get_str(obj, "drive-id", NULL);
> + } else {
> + desc = g_strdup(type);
> + }
Ugh. Comparing the device name to an incomplete set of strings here and
then figuring out for each what the specific way for this device is to
create a nice string sounds like a bad idea.
Why can't all devices just expose a property with a human-readable
string? We'll need it for more than just the disk change menus.
And then, of course, the question is still what a good human-readable
string is. A serial number generated by qemu, as we now get by default
for the CD-ROM, probably isn't. Something like "ATAPI CD-ROM at secondary
master" would probably be more helpful in this case.
> +
> + if (strcmp(type, "ide-cd") == 0) {
> + disk_type = DT_CDROM;
> + } else if (strcmp(type, "isa-fdc") == 0) {
> + disk_type = DT_FLOPPY;
> + } else {
> + disk_type = DT_NORMAL;
> + }
Same thing here, comparing against strings is a hack. Devices should
probably have a property that says what kind of device they are.
> + bdm = g_malloc0(sizeof(*bdm));
> + bdm->name = g_strdup(block_id);
> + bdm->path = g_strdup(path);
> + bdm->desc = desc;
> + bdm->disk_type = disk_type;
> +
> + g_free(type);
> +
> + g_hash_table_insert(s->devices_map, bdm->name, bdm);
> + }
> + g_free(block_id);
> +}
Kevin
- Re: [Qemu-devel] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices,
Kevin Wolf <=
- Re: [Qemu-devel] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices, Anthony Liguori, 2013/05/02
- Re: [Qemu-devel] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices, Luiz Capitulino, 2013/05/02
- Re: [Qemu-devel] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices, Kevin Wolf, 2013/05/02
- Re: [Qemu-devel] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices, Markus Armbruster, 2013/05/02
- Re: [Qemu-devel] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices, Anthony Liguori, 2013/05/02
- Re: [Qemu-devel] [libvirt] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices, Daniel P. Berrange, 2013/05/03
- Re: [Qemu-devel] [libvirt] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices, Anthony Liguori, 2013/05/03
- Re: [Qemu-devel] [libvirt] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices, Daniel P. Berrange, 2013/05/03
- Re: [Qemu-devel] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices, Anthony Liguori, 2013/05/02