On Fri, Jan 21, 2011 at 05:19:13PM -0500, Chunqiang Tang wrote: > This patch adds the 'update' command to qemu-img. FVD stores various > image-specific configurable parameters in the image header. A user can use > 'qemu-img update' to modify those parameters and accordingly controls FVD's > runtime behavior. This command may also be leveraged by other block device > drivers, e.g., to set the size of the in-memory metadata cache. Currently > those parameters are hard-coded in a one-size-fit-all manner.
There's a high risk that users will try this command while the VM is running. A safe-guard is needed here in order to avoid corrupting the image. Please use qemu-option.h instead of int argc, char **argv just like qemu-img create -o does. Finally, is this interface really necessary? As a developer it can be useful to tweak image values (in QED I actually have a free-standing tool that can query and manipulate image internals). But should users need to micromanage every image file in order to achieve desired functionality/performance? What's the real need here? > diff --git a/qemu-img.c b/qemu-img.c > index afd9ed2..5f35c4d 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -1054,6 +1054,49 @@ static int img_info(int argc, char **argv) > return 0; > } > > +static int img_update(int argc, char **argv) > +{ > + int c; > + const char *filename, *fmt; > + BlockDriverState *bs; > + > + fmt = NULL; > + for(;;) { > + c = getopt(argc, argv, "f:h"); > + if (c == -1) > + break; {}, see CODING_STYLE and HACKING. > + switch(c) { > + case 'h': > + help(); > + break; > + case 'f': > + fmt = optarg; > + break; > + } > + } > + if (optind >= argc) > + help(); {} > + filename = argv[optind++]; > + bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING > + | BDRV_O_RDWR); > + if (!bs) { > + return 1; > + } > + > + if (bs->drv->bdrv_update) { > + bs->drv->bdrv_update(bs, argc-optind, &argv[optind]); > + } > + else { } else {, see CODING_STYLE > + char fmt_name[128]; > + bdrv_get_format(bs, fmt_name, sizeof(fmt_name)); > + error_report ("The 'update' command is not supported for " > + "the '%s' image format.", fmt_name); Return value should be 1? Stefan