| Message ID | 1317658142-21383-2-git-send-email-fsimonce@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 10/03/2011 06:09 PM, Federico Simoncelli wrote: > Add the new option [-n] for snapshot_blkdev to avoid the image creation. > The file provided as [new-image-file] is considered as already initialized > and will be used after passing a check for the backing file. Seems ok to me as a way to go around fdget and still have selinux gain. Worth to get Kevin's view too. Federico, would you like to ack or extend the design: http://wiki.qemu.org/Features/Snapshots > > Signed-off-by: Federico Simoncelli<fsimonce@redhat.com> > --- > blockdev.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- > hmp-commands.hx | 7 ++++--- > qmp-commands.hx | 4 ++-- > 3 files changed, 58 insertions(+), 7 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index 0827bf7..bd46808 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -550,8 +550,53 @@ void do_commit(Monitor *mon, const QDict *qdict) > } > } > > +static int check_snapshot_file(const char *filename, const char *oldfilename, > + int flags, BlockDriver *drv) > +{ > + BlockDriverState *bs; > + char bak_filename[1024], *abs_filename; > + int ret = 0; > + > + bs = bdrv_new(""); > + if (!bs) { > + return -1; > + } > + > + ret = bdrv_open(bs, filename, flags, drv); > + if (ret) { > + qerror_report(QERR_OPEN_FILE_FAILED, filename); > + goto err0; > + } > + > + if (bs->backing_file) { > + path_combine(bak_filename, sizeof(bak_filename), > + filename, bs->backing_file); > + > + abs_filename = realpath(bak_filename, NULL); > + if (!abs_filename) { > + ret = -1; > + goto err1; > + } > + > + if (strcmp(abs_filename, oldfilename)) { > + qerror_report(QERR_OPEN_FILE_FAILED, filename); > + ret = -1; > + } > + > + free(abs_filename); > + } > + > +err1: > + bdrv_close(bs); > + > +err0: > + bdrv_delete(bs); > + return ret; > +} > + > int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data) > { > + const int nocreate = qdict_get_try_bool(qdict, "nocreate", 0); > const char *device = qdict_get_str(qdict, "device"); > const char *filename = qdict_get_try_str(qdict, "snapshot-file"); > const char *format = qdict_get_try_str(qdict, "format"); > @@ -597,8 +642,13 @@ int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data) > goto out; > } > > - ret = bdrv_img_create(filename, format, bs->filename, > - bs->drv->format_name, NULL, -1, flags); > + if (nocreate) { > + ret = check_snapshot_file(filename, bs->filename, flags, drv); > + } else { > + ret = bdrv_img_create(filename, format, bs->filename, > + bs->drv->format_name, NULL, -1, flags); > + } > + > if (ret) { > goto out; > } > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 9e1cca8..eb9fcd4 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -840,11 +840,12 @@ ETEXI > > { > .name = "snapshot_blkdev", > - .args_type = "device:B,snapshot-file:s?,format:s?", > - .params = "device [new-image-file] [format]", > + .args_type = "nocreate:-n,device:B,snapshot-file:s?,format:s?", > + .params = "[-n] device [new-image-file] [format]", > .help = "initiates a live snapshot\n\t\t\t" > "of device. If a new image file is specified, the\n\t\t\t" > - "new image file will become the new root image.\n\t\t\t" > + "new image file will be created (unless -n is\n\t\t\t" > + "specified) and will become the new root image.\n\t\t\t" > "If format is specified, the snapshot file will\n\t\t\t" > "be created in that format. Otherwise the\n\t\t\t" > "snapshot will be internal! (currently unsupported)", > diff --git a/qmp-commands.hx b/qmp-commands.hx > index d83bce5..7af36d8 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -695,8 +695,8 @@ EQMP > > { > .name = "blockdev-snapshot-sync", > - .args_type = "device:B,snapshot-file:s?,format:s?", > - .params = "device [new-image-file] [format]", > + .args_type = "nocreate:-n,device:B,snapshot-file:s?,format:s?", > + .params = "[-n] device [new-image-file] [format]", > .user_print = monitor_user_noop, > .mhandler.cmd_new = do_snapshot_blkdev, > },
----- Original Message ----- > From: "Dor Laor" <dlaor@redhat.com> > To: "Federico Simoncelli" <fsimonce@redhat.com> > Cc: qemu-devel@nongnu.org, abaron@redhat.com, "Kevin Wolf" <kwolf@redhat.com> > Sent: Friday, October 7, 2011 12:45:06 AM > Subject: Re: [Qemu-devel] [PATCH] qemu: new option for snapshot_blkdev to avoid image creation > > On 10/03/2011 06:09 PM, Federico Simoncelli wrote: > > Add the new option [-n] for snapshot_blkdev to avoid the image > > creation. > > The file provided as [new-image-file] is considered as already > > initialized > > and will be used after passing a check for the backing file. > > Seems ok to me as a way to go around fdget and still have selinux > gain. > Worth to get Kevin's view too. Hi Kevin, any news on this? > Federico, would you like to ack or extend the design: > http://wiki.qemu.org/Features/Snapshots Dor, should I do it now or only after my patch is acked?
On 10/11/2011 01:56 PM, Federico Simoncelli wrote: > ----- Original Message ----- >> From: "Dor Laor"<dlaor@redhat.com> >> To: "Federico Simoncelli"<fsimonce@redhat.com> >> Cc: qemu-devel@nongnu.org, abaron@redhat.com, "Kevin Wolf"<kwolf@redhat.com> >> Sent: Friday, October 7, 2011 12:45:06 AM >> Subject: Re: [Qemu-devel] [PATCH] qemu: new option for snapshot_blkdev to avoid image creation >> >> On 10/03/2011 06:09 PM, Federico Simoncelli wrote: >>> Add the new option [-n] for snapshot_blkdev to avoid the image >>> creation. >>> The file provided as [new-image-file] is considered as already >>> initialized >>> and will be used after passing a check for the backing file. >> >> Seems ok to me as a way to go around fdget and still have selinux >> gain. >> Worth to get Kevin's view too. > > Hi Kevin, any news on this? > >> Federico, would you like to ack or extend the design: >> http://wiki.qemu.org/Features/Snapshots > > Dor, should I do it now or only after my patch is acked? We're not that strict in the OSS world ;) you can do it before and mark as a pending request. btw: Kevin is back so he will probably drain his queue soon.
diff --git a/blockdev.c b/blockdev.c index 0827bf7..bd46808 100644 --- a/blockdev.c +++ b/blockdev.c @@ -550,8 +550,53 @@ void do_commit(Monitor *mon, const QDict *qdict) } } +static int check_snapshot_file(const char *filename, const char *oldfilename, + int flags, BlockDriver *drv) +{ + BlockDriverState *bs; + char bak_filename[1024], *abs_filename; + int ret = 0; + + bs = bdrv_new(""); + if (!bs) { + return -1; + } + + ret = bdrv_open(bs, filename, flags, drv); + if (ret) { + qerror_report(QERR_OPEN_FILE_FAILED, filename); + goto err0; + } + + if (bs->backing_file) { + path_combine(bak_filename, sizeof(bak_filename), + filename, bs->backing_file); + + abs_filename = realpath(bak_filename, NULL); + if (!abs_filename) { + ret = -1; + goto err1; + } + + if (strcmp(abs_filename, oldfilename)) { + qerror_report(QERR_OPEN_FILE_FAILED, filename); + ret = -1; + } + + free(abs_filename); + } + +err1: + bdrv_close(bs); + +err0: + bdrv_delete(bs); + return ret; +} + int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data) { + const int nocreate = qdict_get_try_bool(qdict, "nocreate", 0); const char *device = qdict_get_str(qdict, "device"); const char *filename = qdict_get_try_str(qdict, "snapshot-file"); const char *format = qdict_get_try_str(qdict, "format"); @@ -597,8 +642,13 @@ int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data) goto out; } - ret = bdrv_img_create(filename, format, bs->filename, - bs->drv->format_name, NULL, -1, flags); + if (nocreate) { + ret = check_snapshot_file(filename, bs->filename, flags, drv); + } else { + ret = bdrv_img_create(filename, format, bs->filename, + bs->drv->format_name, NULL, -1, flags); + } + if (ret) { goto out; } diff --git a/hmp-commands.hx b/hmp-commands.hx index 9e1cca8..eb9fcd4 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -840,11 +840,12 @@ ETEXI { .name = "snapshot_blkdev", - .args_type = "device:B,snapshot-file:s?,format:s?", - .params = "device [new-image-file] [format]", + .args_type = "nocreate:-n,device:B,snapshot-file:s?,format:s?", + .params = "[-n] device [new-image-file] [format]", .help = "initiates a live snapshot\n\t\t\t" "of device. If a new image file is specified, the\n\t\t\t" - "new image file will become the new root image.\n\t\t\t" + "new image file will be created (unless -n is\n\t\t\t" + "specified) and will become the new root image.\n\t\t\t" "If format is specified, the snapshot file will\n\t\t\t" "be created in that format. Otherwise the\n\t\t\t" "snapshot will be internal! (currently unsupported)", diff --git a/qmp-commands.hx b/qmp-commands.hx index d83bce5..7af36d8 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -695,8 +695,8 @@ EQMP { .name = "blockdev-snapshot-sync", - .args_type = "device:B,snapshot-file:s?,format:s?", - .params = "device [new-image-file] [format]", + .args_type = "nocreate:-n,device:B,snapshot-file:s?,format:s?", + .params = "[-n] device [new-image-file] [format]", .user_print = monitor_user_noop, .mhandler.cmd_new = do_snapshot_blkdev, },
Add the new option [-n] for snapshot_blkdev to avoid the image creation. The file provided as [new-image-file] is considered as already initialized and will be used after passing a check for the backing file. Signed-off-by: Federico Simoncelli <fsimonce@redhat.com> --- blockdev.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- hmp-commands.hx | 7 ++++--- qmp-commands.hx | 4 ++-- 3 files changed, 58 insertions(+), 7 deletions(-)