Message ID | 1343424728-22461-13-git-send-email-lcapitulino@redhat.com |
---|---|
State | New |
Headers | show |
Luiz Capitulino <lcapitulino@redhat.com> 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 <lcapitulino@redhat.com> > --- > 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); > }
Markus Armbruster <armbru@redhat.com> writes: > Luiz Capitulino <lcapitulino@redhat.com> 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 <lcapitulino@redhat.com> >> --- >> 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))? Those are not QMP functions. hmp.c is only allowed to use QMP interfaces. Regards, Anthony Liguori
On Wed, 01 Aug 2012 14:39:04 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Luiz Capitulino <lcapitulino@redhat.com> 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 <lcapitulino@redhat.com> > > --- > > 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 :) I don't remember why I did it, but I've fixed for v1. Btw, this patch is different v1 because I'm dropping error_is_type() usage (like I did for the previous patch). Btw 2, I'm assuming that the other questions you asked have already been answered in other emails. > > > return; > > } > > + > > hmp_handle_error(mon, &err); > > } >
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); + 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)); return; } + hmp_handle_error(mon, &err); }
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 <lcapitulino@redhat.com> --- hmp.c | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-)