| Message ID | 1295474688-6219-1-git-send-email-ctang@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jan 19, 2011 at 05:04:44PM -0500, Chunqiang Tang wrote: > Part 1 of the block device driver for the proposed FVD image format. > Multiple patches are used in order to manage the size of each patch. > This patch includes existing files that are modified by FVD. Please try to split the patches into logical parts, and use descriptive subject lines for each patch. E.g. adding the new sim command to qemu-io could be one patch, adding the img_update (why not just update?) command to qemu-img another, moving code into qemu-tool-time.c one more, etc. > - > + Please do not introduce random whitespace changes in patches.
> Please try to split the patches into logical parts, and use descriptive > subject lines for each patch. > E.g. adding the new sim command to qemu-io could be one patch, adding > the img_update (why not just update?) command to qemu-img another, > moving code into qemu-tool-time.c one more, etc. Will do and thank you for the detailed instructions. > > - > > + > Please do not introduce random whitespace changes in patches. Stefan Weil previously suggested removing spaces at the end of a line, and I used a script to do that. It seems that in this example, the old code has multiple spaces on an empty line, which were automatically removed by the script.
Am 20.01.2011 15:49, schrieb Chunqiang Tang: >> Please try to split the patches into logical parts, and use descriptive >> subject lines for each patch. >> E.g. adding the new sim command to qemu-io could be one patch, adding >> the img_update (why not just update?) command to qemu-img another, >> moving code into qemu-tool-time.c one more, etc. > > Will do and thank you for the detailed instructions. > >>> - >>> + >> Please do not introduce random whitespace changes in patches. > > Stefan Weil previously suggested removing spaces at the end of a line, > and > I used a script to do that. It seems that in this example, the old code > has multiple spaces on an empty line, which were automatically removed by > the script. Yes, that's a problem with some parts of the old code. For files which you want to modify, you could remove the spaces with your script before applying your other modifications and create a separate patch which only removes the superfluous spaces. So your patch series would start with patches which only remove spaces at line endings (and say so in the patch descriptions). Then these changes are no longer random whitespace changes. Regards, Stefan Weil
On 01/19/2011 04:04 PM, Chunqiang Tang wrote: > Part 1 of the block device driver for the proposed FVD image format. > Multiple patches are used in order to manage the size of each patch. > This patch includes existing files that are modified by FVD. > > See the related discussions at > http://lists.gnu.org/archive/html/qemu-devel/2011-01/msg00426.html . > > Signed-off-by: Chunqiang Tang<ctang@us.ibm.com> > --- > Makefile | 10 +++++--- > Makefile.objs | 1 + > block.c | 12 +++++----- > block_int.h | 5 ++- > configure | 2 +- > qemu-img-cmds.hx | 6 +++++ > qemu-img.c | 62 ++++++++++++++++++++++++++++++++++++++++++++--------- > qemu-io.c | 3 ++ > qemu-option.c | 4 +++ > qemu-tool.c | 36 ------------------------------- > 10 files changed, 81 insertions(+), 60 deletions(-) > > diff --git a/Makefile b/Makefile > index 6d601ee..da4d777 100644 > --- a/Makefile > +++ b/Makefile > @@ -151,13 +151,15 @@ version-obj-$(CONFIG_WIN32) += version.o > ###################################################################### > > qemu-img.o: qemu-img-cmds.h > -qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o: $(GENERATED_HEADERS) > +qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o qemu-test.o: $(GENERATED_HEADERS) > > -qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o > +qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-tool-time.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o > > -qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o > +qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-tool-time.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o > > -qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o > +qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-tool-time.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o > + > +qemu-test$(EXESUF): qemu-test.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o > > qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx > $(call quiet-command,sh $(SRC_PATH)/hxtool -h< $< > $@," GEN $@") > diff --git a/Makefile.objs b/Makefile.objs > index c3e52c5..c0c1155 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -23,6 +23,7 @@ block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o > block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o > block-nested-y += qed-check.o > block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o > +block-nested-y += blksim.o fvd.o > block-nested-$(CONFIG_WIN32) += raw-win32.o > block-nested-$(CONFIG_POSIX) += raw-posix.o > block-nested-$(CONFIG_CURL) += curl.o > diff --git a/block.c b/block.c > index ff2795b..856bb1a 100644 > --- a/block.c > +++ b/block.c > @@ -58,7 +58,7 @@ static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num, > static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num, > const uint8_t *buf, int nb_sectors); > > -static QTAILQ_HEAD(, BlockDriverState) bdrv_states = > +QTAILQ_HEAD(, BlockDriverState) bdrv_states = > QTAILQ_HEAD_INITIALIZER(bdrv_states); > This looks suspicious and indicates your doing something bad. > > static QLIST_HEAD(, BlockDriver) bdrv_drivers = > @@ -768,7 +768,7 @@ int bdrv_commit(BlockDriverState *bs) > > if (!drv) > return -ENOMEDIUM; > - > + > if (!bs->backing_hd) { > return -ENOTSUP; > } > @@ -1538,7 +1538,7 @@ int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors) > * 'nb_sectors' is the max value 'pnum' should be set to. > */ > int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors, > - int *pnum) > + int *pnum) > { > int64_t n; > if (!bs->drv->bdrv_is_allocated) { > @@ -2050,9 +2050,9 @@ BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num, > cb, opaque); > > if (ret) { > - /* Update stats even though technically transfer has not happened. */ > - bs->rd_bytes += (unsigned) nb_sectors * BDRV_SECTOR_SIZE; > - bs->rd_ops ++; > + /* Update stats even though technically transfer has not happened. */ > + bs->rd_bytes += (unsigned) nb_sectors * BDRV_SECTOR_SIZE; > + bs->rd_ops ++; > } > > return ret; > diff --git a/block_int.h b/block_int.h > index 12663e8..2343d07 100644 > --- a/block_int.h > +++ b/block_int.h > @@ -28,8 +28,8 @@ > #include "qemu-option.h" > #include "qemu-queue.h" > > -#define BLOCK_FLAG_ENCRYPT 1 > -#define BLOCK_FLAG_COMPAT6 4 > +#define BLOCK_FLAG_ENCRYPT 1 > +#define BLOCK_FLAG_COMPAT6 4 > > #define BLOCK_OPT_SIZE "size" > #define BLOCK_OPT_ENCRYPT "encryption" > @@ -98,6 +98,7 @@ struct BlockDriver { > int (*bdrv_snapshot_load_tmp)(BlockDriverState *bs, > const char *snapshot_name); > int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi); > + int (*bdrv_update)(BlockDriverState *bs, int argc, char **argv); > argc/argv is an awful interface because the semantics end up varying widely. If we want to support changing disk format parameters, we should use a structured option format so we can ensure it's exposed to the user in a consistent way. IOW, a size is always parsed as <integer>[SUFFIX] and not 8 different variations of that theme. > bs = qemu_mallocz(bs_n * sizeof(BlockDriverState *)); > > total_sectors = 0; > @@ -865,7 +865,7 @@ static int img_convert(int argc, char **argv) > assume that sectors which are unallocated in the input image > are present in both the output's and input's base images (no > need to copy them). */ > - if (out_baseimg) { > + if (out_baseimg || bs[bs_i]->backing_file[0]==0) { > This looks like a bug fix of some sort and should be it's own patch with an explanation. > if (!bdrv_is_allocated(bs[bs_i], sector_num - bs_offset, > n,&n1)) { > sector_num += n1; > @@ -941,10 +941,10 @@ static int64_t get_allocated_file_size(const char *filename) > /* WinNT support GetCompressedFileSize to determine allocate size */ > get_compressed = (get_compressed_t) GetProcAddress(GetModuleHandle("kernel32"), "GetCompressedFileSizeA"); > if (get_compressed) { > - DWORD high, low; > - low = get_compressed(filename,&high); > - if (low != 0xFFFFFFFFlu || GetLastError() == NO_ERROR) > - return (((int64_t) high)<< 32) + low; > + DWORD high, low; > + low = get_compressed(filename,&high); > + if (low != 0xFFFFFFFFlu || GetLastError() == NO_ERROR) > + return (((int64_t) high)<< 32) + low; > } > > if (_stati64(filename,&st)< 0) > @@ -1036,11 +1036,6 @@ static int img_info(int argc, char **argv) > if (bdrv_is_encrypted(bs)) { > printf("encrypted: yes\n"); > } > - if (bdrv_get_info(bs,&bdi)>= 0) { > - if (bdi.cluster_size != 0) { > - printf("cluster_size: %d\n", bdi.cluster_size); > - } > - } > bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename)); > if (backing_filename[0] != '\0') { > path_combine(backing_filename2, sizeof(backing_filename2), > @@ -1049,11 +1044,56 @@ static int img_info(int argc, char **argv) > backing_filename, > backing_filename2); > } > + if (bdrv_get_info(bs,&bdi)>= 0) { > + if (bdi.cluster_size != 0) > + printf("cluster_size: %d\n", bdi.cluster_size); > + } > dump_snapshots(bs); > bdrv_delete(bs); > 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; > + 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==NULL) { > + 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); > + } > + > + bs->drv->bdrv_update(bs, argc-optind,&argv[optind]); > + > + bdrv_delete(bs); > + return 0; > +} > + > #define SNAPSHOT_LIST 1 > #define SNAPSHOT_CREATE 2 > #define SNAPSHOT_APPLY 3 > diff --git a/qemu-io.c b/qemu-io.c > index 5b24c5e..c32f8d4 100644 > --- a/qemu-io.c > +++ b/qemu-io.c > @@ -1701,6 +1701,8 @@ init_check_command( > return 1; > } > > +#include "qemu-io-sim.c" > + > 1) I don't see qemu-io-sim.c in this patch which means this breaks the build 2) Including C files is evil. > static void usage(const char *name) > { > printf( > @@ -1807,6 +1809,7 @@ int main(int argc, char **argv) > add_command(&discard_cmd); > add_command(&alloc_cmd); > add_command(&map_cmd); > + add_command(&sim_cmd); > > add_args_command(init_args_command); > add_check_command(init_check_command); > diff --git a/qemu-option.c b/qemu-option.c > index 65db542..10ef45f 100644 > --- a/qemu-option.c > +++ b/qemu-option.c > @@ -289,6 +289,10 @@ int set_option_parameter(QEMUOptionParameter *list, const char *name, > return -1; > break; > > + case OPT_NUMBER: > + list->value.n = atoi (value); > + break; > + > default: > fprintf(stderr, "Bug: Option '%s' has an unknown type\n", name); > return -1; > diff --git a/qemu-tool.c b/qemu-tool.c > index 392e1c9..fdcb2f8 100644 > --- a/qemu-tool.c > +++ b/qemu-tool.c > @@ -23,12 +23,6 @@ QEMUClock *rt_clock; > > FILE *logfile; > > -struct QEMUBH > -{ > - QEMUBHFunc *cb; > - void *opaque; > -}; > - > void qemu_service_io(void) > { > } > @@ -73,36 +67,6 @@ void monitor_protocol_event(MonitorEvent event, QObject *data) > { > } > > -QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque) > -{ > - QEMUBH *bh; > - > - bh = qemu_malloc(sizeof(*bh)); > - bh->cb = cb; > - bh->opaque = opaque; > - > - return bh; > -} > - > -int qemu_bh_poll(void) > -{ > - return 0; > -} > - > -void qemu_bh_schedule(QEMUBH *bh) > -{ > - bh->cb(bh->opaque); > -} > - > -void qemu_bh_cancel(QEMUBH *bh) > -{ > -} > - > -void qemu_bh_delete(QEMUBH *bh) > -{ > - qemu_free(bh); > -} > - > int qemu_set_fd_handler2(int fd, > IOCanReadHandler *fd_read_poll, > IOHandler *fd_read, > These functions surely cannot just be deleted like this. Regards, Anthony Liguori
> > -void qemu_bh_schedule(QEMUBH *bh) > > -{ > > - bh->cb(bh->opaque); > > -} > > - > > -void qemu_bh_cancel(QEMUBH *bh) > > -{ > > -} > > - > > -void qemu_bh_delete(QEMUBH *bh) > > -{ > > - qemu_free(bh); > > -} > > - > > int qemu_set_fd_handler2(int fd, > > IOCanReadHandler *fd_read_poll, > > IOHandler *fd_read, > > > > These functions surely cannot just be deleted like this. These functions were not deleted but instead moved into a separate file qemu-tool-time.c, because those functions are time related and the implementations are different in the simulation mode and in the real mode. In the latest patches, these functions are kept in qemu-tool.c but their implementations support both cases based on a switch.
On 20 January 2011 17:08, Stefan Weil <weil@mail.berlios.de> wrote: > Yes, that's a problem with some parts of the old code. > For files which you want to modify, you could remove > the spaces with your script before applying your other > modifications and create a separate patch which only > removes the superfluous spaces. (This kind of came up in the other thread about fixing non-C89 comments. I don't have any particular interest in this area of the qemu source so this is a general remark.) I definitely dislike patches which change whitespace or indentation for an entire file, even if they are standalone "only fixing whitespace" patches; they make it much harder to deal with forks and branches of qemu. I would prefer it if we only fix whitespace, indent and bracing for lines we're touching anyway. -- PMM
On 01/21/2011 08:51 PM, Chunqiang Tang wrote: >>> -void qemu_bh_schedule(QEMUBH *bh) >>> -{ >>> - bh->cb(bh->opaque); >>> -} >>> - >>> -void qemu_bh_cancel(QEMUBH *bh) >>> -{ >>> -} >>> - >>> -void qemu_bh_delete(QEMUBH *bh) >>> -{ >>> - qemu_free(bh); >>> -} >>> - >>> int qemu_set_fd_handler2(int fd, >>> IOCanReadHandler *fd_read_poll, >>> IOHandler *fd_read, >>> >>> >> These functions surely cannot just be deleted like this. >> > These functions were not deleted but instead moved into a separate file > qemu-tool-time.c, because those functions are time related and the > implementations are different in the simulation mode and in the real mode. > In the latest patches, these functions are kept in qemu-tool.c but their > implementations support both cases based on a switch. > I think the root of the problem is that your series didn't maintain bisectability. IOW, each patch needs to be able to be applied one at a time such that at each point, the build doesn't break and functionality doesn't break. Otherwise, tools like git bisect don't work. Regards, Anthony Liguori
> I think the root of the problem is that your series didn't maintain > bisectability. > > IOW, each patch needs to be able to be applied one at a time such that > at each point, the build doesn't break and functionality doesn't break. > > Otherwise, tools like git bisect don't work. This was true with the old, big FVD patches you reviewed. Following Christoph Hellwig's suggestion, the new series of FVD patches submitted last Friday, e.g., "FVD: Added the simulated 'blksim' driver", add individual smaller functions and breaks neither compilation nor execution.
> On 20 January 2011 17:08, Stefan Weil <weil@mail.berlios.de> wrote: > > Yes, that's a problem with some parts of the old code. > > For files which you want to modify, you could remove > > the spaces with your script before applying your other > > modifications and create a separate patch which only > > removes the superfluous spaces. > > (This kind of came up in the other thread about fixing > non-C89 comments. I don't have any particular interest in > this area of the qemu source so this is a general remark.) > > I definitely dislike patches which change whitespace or > indentation for an entire file, even if they are standalone > "only fixing whitespace" patches; they make it much harder > to deal with forks and branches of qemu. I would prefer > it if we only fix whitespace, indent and bracing for lines > we're touching anyway. I agree with this, i.e., only fixing the lines we are touching anyway. This is the approach the new series of FVD patches took.
On 01/24/11 15:50, Chunqiang Tang wrote: >> I think the root of the problem is that your series didn't maintain >> bisectability. >> >> IOW, each patch needs to be able to be applied one at a time such that >> at each point, the build doesn't break and functionality doesn't break. >> >> Otherwise, tools like git bisect don't work. > > This was true with the old, big FVD patches you reviewed. Following > Christoph Hellwig's suggestion, the new series of FVD patches submitted > last Friday, e.g., "FVD: Added the simulated 'blksim' driver", add > individual smaller functions and breaks neither compilation nor execution. Then you need to break up the new patch into smaller chunks and make sure they can each be applied without breaking the build. Having a separate patch that moves functions to another file, because they are too be shared, is a completely valid patch. Jes
diff --git a/Makefile b/Makefile index 6d601ee..da4d777 100644 --- a/Makefile +++ b/Makefile @@ -151,13 +151,15 @@ version-obj-$(CONFIG_WIN32) += version.o ###################################################################### qemu-img.o: qemu-img-cmds.h -qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o: $(GENERATED_HEADERS) +qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o qemu-test.o: $(GENERATED_HEADERS) -qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o +qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-tool-time.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o -qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o +qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-tool-time.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o -qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o +qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-tool-time.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o + +qemu-test$(EXESUF): qemu-test.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx $(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@," GEN $@") diff --git a/Makefile.objs b/Makefile.objs index c3e52c5..c0c1155 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -23,6 +23,7 @@ block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o block-nested-y += qed-check.o block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o +block-nested-y += blksim.o fvd.o block-nested-$(CONFIG_WIN32) += raw-win32.o block-nested-$(CONFIG_POSIX) += raw-posix.o block-nested-$(CONFIG_CURL) += curl.o diff --git a/block.c b/block.c index ff2795b..856bb1a 100644 --- a/block.c +++ b/block.c @@ -58,7 +58,7 @@ static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num, static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num, const uint8_t *buf, int nb_sectors); -static QTAILQ_HEAD(, BlockDriverState) bdrv_states = +QTAILQ_HEAD(, BlockDriverState) bdrv_states = QTAILQ_HEAD_INITIALIZER(bdrv_states); static QLIST_HEAD(, BlockDriver) bdrv_drivers = @@ -768,7 +768,7 @@ int bdrv_commit(BlockDriverState *bs) if (!drv) return -ENOMEDIUM; - + if (!bs->backing_hd) { return -ENOTSUP; } @@ -1538,7 +1538,7 @@ int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors) * 'nb_sectors' is the max value 'pnum' should be set to. */ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors, - int *pnum) + int *pnum) { int64_t n; if (!bs->drv->bdrv_is_allocated) { @@ -2050,9 +2050,9 @@ BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num, cb, opaque); if (ret) { - /* Update stats even though technically transfer has not happened. */ - bs->rd_bytes += (unsigned) nb_sectors * BDRV_SECTOR_SIZE; - bs->rd_ops ++; + /* Update stats even though technically transfer has not happened. */ + bs->rd_bytes += (unsigned) nb_sectors * BDRV_SECTOR_SIZE; + bs->rd_ops ++; } return ret; diff --git a/block_int.h b/block_int.h index 12663e8..2343d07 100644 --- a/block_int.h +++ b/block_int.h @@ -28,8 +28,8 @@ #include "qemu-option.h" #include "qemu-queue.h" -#define BLOCK_FLAG_ENCRYPT 1 -#define BLOCK_FLAG_COMPAT6 4 +#define BLOCK_FLAG_ENCRYPT 1 +#define BLOCK_FLAG_COMPAT6 4 #define BLOCK_OPT_SIZE "size" #define BLOCK_OPT_ENCRYPT "encryption" @@ -98,6 +98,7 @@ struct BlockDriver { int (*bdrv_snapshot_load_tmp)(BlockDriverState *bs, const char *snapshot_name); int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi); + int (*bdrv_update)(BlockDriverState *bs, int argc, char **argv); int (*bdrv_save_vmstate)(BlockDriverState *bs, const uint8_t *buf, int64_t pos, int size); diff --git a/configure b/configure index d68f862..35b29e9 100755 --- a/configure +++ b/configure @@ -2362,7 +2362,7 @@ confdir=$sysconfdir$confsuffix tools= if test "$softmmu" = yes ; then - tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools" + tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) qemu-test\$(EXESUF) $tools" if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then tools="qemu-nbd\$(EXESUF) $tools" if [ "$check_utests" = "yes" ]; then diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx index 6c7176f..1ad378b 100644 --- a/qemu-img-cmds.hx +++ b/qemu-img-cmds.hx @@ -39,6 +39,12 @@ STEXI @item info [-f @var{fmt}] @var{filename} ETEXI +DEF("update", img_update, + "update [-f fmt] filename [attr1=val1 attr2=val2 ...]") +STEXI +@item update [-f @var{fmt}] @var{filename} [@var{attr1=val1 attr2=val2 ...}]") +ETEXI + DEF("snapshot", img_snapshot, "snapshot [-l | -a snapshot | -c snapshot | -d snapshot] filename") STEXI diff --git a/qemu-img.c b/qemu-img.c index afd9ed2..1694206 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -637,7 +637,7 @@ static int img_convert(int argc, char **argv) ret = -1; goto out; } - + bs = qemu_mallocz(bs_n * sizeof(BlockDriverState *)); total_sectors = 0; @@ -865,7 +865,7 @@ static int img_convert(int argc, char **argv) assume that sectors which are unallocated in the input image are present in both the output's and input's base images (no need to copy them). */ - if (out_baseimg) { + if (out_baseimg || bs[bs_i]->backing_file[0]==0) { if (!bdrv_is_allocated(bs[bs_i], sector_num - bs_offset, n, &n1)) { sector_num += n1; @@ -941,10 +941,10 @@ static int64_t get_allocated_file_size(const char *filename) /* WinNT support GetCompressedFileSize to determine allocate size */ get_compressed = (get_compressed_t) GetProcAddress(GetModuleHandle("kernel32"), "GetCompressedFileSizeA"); if (get_compressed) { - DWORD high, low; - low = get_compressed(filename, &high); - if (low != 0xFFFFFFFFlu || GetLastError() == NO_ERROR) - return (((int64_t) high) << 32) + low; + DWORD high, low; + low = get_compressed(filename, &high); + if (low != 0xFFFFFFFFlu || GetLastError() == NO_ERROR) + return (((int64_t) high) << 32) + low; } if (_stati64(filename, &st) < 0) @@ -1036,11 +1036,6 @@ static int img_info(int argc, char **argv) if (bdrv_is_encrypted(bs)) { printf("encrypted: yes\n"); } - if (bdrv_get_info(bs, &bdi) >= 0) { - if (bdi.cluster_size != 0) { - printf("cluster_size: %d\n", bdi.cluster_size); - } - } bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename)); if (backing_filename[0] != '\0') { path_combine(backing_filename2, sizeof(backing_filename2), @@ -1049,11 +1044,56 @@ static int img_info(int argc, char **argv) backing_filename, backing_filename2); } + if (bdrv_get_info(bs, &bdi) >= 0) { + if (bdi.cluster_size != 0) + printf("cluster_size: %d\n", bdi.cluster_size); + } dump_snapshots(bs); bdrv_delete(bs); 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; + 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==NULL) { + 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); + } + + bs->drv->bdrv_update(bs, argc-optind, &argv[optind]); + + bdrv_delete(bs); + return 0; +} + #define SNAPSHOT_LIST 1 #define SNAPSHOT_CREATE 2 #define SNAPSHOT_APPLY 3 diff --git a/qemu-io.c b/qemu-io.c index 5b24c5e..c32f8d4 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -1701,6 +1701,8 @@ init_check_command( return 1; } +#include "qemu-io-sim.c" + static void usage(const char *name) { printf( @@ -1807,6 +1809,7 @@ int main(int argc, char **argv) add_command(&discard_cmd); add_command(&alloc_cmd); add_command(&map_cmd); + add_command(&sim_cmd); add_args_command(init_args_command); add_check_command(init_check_command); diff --git a/qemu-option.c b/qemu-option.c index 65db542..10ef45f 100644 --- a/qemu-option.c +++ b/qemu-option.c @@ -289,6 +289,10 @@ int set_option_parameter(QEMUOptionParameter *list, const char *name, return -1; break; + case OPT_NUMBER: + list->value.n = atoi (value); + break; + default: fprintf(stderr, "Bug: Option '%s' has an unknown type\n", name); return -1; diff --git a/qemu-tool.c b/qemu-tool.c index 392e1c9..fdcb2f8 100644 --- a/qemu-tool.c +++ b/qemu-tool.c @@ -23,12 +23,6 @@ QEMUClock *rt_clock; FILE *logfile; -struct QEMUBH -{ - QEMUBHFunc *cb; - void *opaque; -}; - void qemu_service_io(void) { } @@ -73,36 +67,6 @@ void monitor_protocol_event(MonitorEvent event, QObject *data) { } -QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque) -{ - QEMUBH *bh; - - bh = qemu_malloc(sizeof(*bh)); - bh->cb = cb; - bh->opaque = opaque; - - return bh; -} - -int qemu_bh_poll(void) -{ - return 0; -} - -void qemu_bh_schedule(QEMUBH *bh) -{ - bh->cb(bh->opaque); -} - -void qemu_bh_cancel(QEMUBH *bh) -{ -} - -void qemu_bh_delete(QEMUBH *bh) -{ - qemu_free(bh); -} - int qemu_set_fd_handler2(int fd, IOCanReadHandler *fd_read_poll, IOHandler *fd_read,
Part 1 of the block device driver for the proposed FVD image format. Multiple patches are used in order to manage the size of each patch. This patch includes existing files that are modified by FVD. See the related discussions at http://lists.gnu.org/archive/html/qemu-devel/2011-01/msg00426.html . Signed-off-by: Chunqiang Tang <ctang@us.ibm.com> --- Makefile | 10 +++++--- Makefile.objs | 1 + block.c | 12 +++++----- block_int.h | 5 ++- configure | 2 +- qemu-img-cmds.hx | 6 +++++ qemu-img.c | 62 ++++++++++++++++++++++++++++++++++++++++++++--------- qemu-io.c | 3 ++ qemu-option.c | 4 +++ qemu-tool.c | 36 ------------------------------- 10 files changed, 81 insertions(+), 60 deletions(-)