From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35013) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwYCk-0000Re-DC for qemu-devel@nongnu.org; Wed, 01 Aug 2012 08:39:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SwYCh-0002eb-8n for qemu-devel@nongnu.org; Wed, 01 Aug 2012 08:39:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:12971) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwYCh-0002eV-05 for qemu-devel@nongnu.org; Wed, 01 Aug 2012 08:39:07 -0400 From: Markus Armbruster References: <1343424728-22461-1-git-send-email-lcapitulino@redhat.com> <1343424728-22461-13-git-send-email-lcapitulino@redhat.com> Date: Wed, 01 Aug 2012 14:39:04 +0200 In-Reply-To: <1343424728-22461-13-git-send-email-lcapitulino@redhat.com> (Luiz Capitulino's message of "Fri, 27 Jul 2012 18:31:53 -0300") Message-ID: <877gtivlav.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 12/27] hmp: hmp_change(): don't use error_get_field() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: kwolf@redhat.com, pbonzini@redhat.com, aliguori@us.ibm.com, eblake@redhat.com, qemu-devel@nongnu.org Luiz Capitulino writes: > Use the 'device' passed by the user and call qmp_query_block() to > get the 'filename' info. > > error_get_field() is going to be dropped by a future commit. > > Signed-off-by: Luiz Capitulino > --- > hmp.c | 37 ++++++++++++++++++++++++++++--------- > 1 file changed, 28 insertions(+), 9 deletions(-) > > diff --git a/hmp.c b/hmp.c > index a906f8a..435c9cd 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -783,17 +783,35 @@ static void hmp_change_read_arg(Monitor *mon, const char *password, > static void cb_hmp_change_bdrv_pwd(Monitor *mon, const char *password, > void *opaque) > { > - Error *encryption_err = opaque; > + char *device = opaque; > Error *err = NULL; > - const char *device; > - > - device = error_get_field(encryption_err, "device"); > > qmp_block_passwd(device, password, &err); > hmp_handle_error(mon, &err); > - error_free(encryption_err); > > monitor_read_command(mon, 1); > + g_free(device); > +} > + > +static char *get_device_file(const char *device) > +{ > + BlockInfoList *bdev_list, *bdev; > + char *ret; > + > + bdev_list = qmp_query_block(NULL); > + for (bdev = bdev_list; bdev; bdev = bdev->next) { > + if (!strcmp(bdev->value->device, device)) { > + break; > + } > + } > + > + assert(bdev); > + assert(bdev->value->has_inserted); > + > + ret = g_strdup(bdev->value->inserted->file); > + qapi_free_BlockInfoList(bdev_list); > + > + return ret; > } > > void hmp_change(Monitor *mon, const QDict *qdict) > @@ -814,9 +832,9 @@ void hmp_change(Monitor *mon, const QDict *qdict) > > qmp_change(device, target, !!arg, arg, &err); > if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) { > - monitor_printf(mon, "%s (%s) is encrypted.\n", > - error_get_field(err, "device"), > - error_get_field(err, "filename")); > + char *filename = get_device_file(device); Elsewhere, we use bdrv_get_encrypted_filename(), which does the right thing for encrypted backing files. Why is that not necessary here? Why not simply bdrv_get_encrypted_filename(bdrv_find(device))? > + monitor_printf(mon, "%s (%s) is encrypted.\n", device, filename); > + g_free(filename); > if (!monitor_get_rs(mon)) { > monitor_printf(mon, > "terminal does not support password prompting\n"); > @@ -824,9 +842,10 @@ void hmp_change(Monitor *mon, const QDict *qdict) > return; > } > readline_start(monitor_get_rs(mon), "Password: ", 1, > - cb_hmp_change_bdrv_pwd, err); > + cb_hmp_change_bdrv_pwd, (void *) g_strdup(device)); Casting a pointer to void * is pointless noise :) > return; > } > + > hmp_handle_error(mon, &err); > }