| Message ID | 1390972061-26560-6-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
The Wednesday 29 Jan 2014 à 13:07:32 (+0800), Fam Zheng wrote : > This is the common but non-trivial steps to assign or change the > backing_hd of BDS. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block.c | 34 +++++++++++++++++++++++++--------- > include/block/block.h | 1 + > 2 files changed, 26 insertions(+), 9 deletions(-) > > diff --git a/block.c b/block.c > index 8000ac0..c4eaa37 100644 > --- a/block.c > +++ b/block.c > @@ -1072,6 +1072,26 @@ fail: > return ret; > } > > +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd) > +{ > + if (bs->backing_hd) { > + bdrv_unref(bs->backing_hd); > + } > + > + bs->backing_hd = backing_hd; > + if (!backing_hd) { > + bs->backing_file[0] = '\0'; > + bs->backing_format[0] = '\0'; > + goto out; > + } > + pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename); > + pstrcpy(bs->backing_format, sizeof(bs->backing_file), I don't understand the sizeof(bs->backing_file). Is it sizeof(bs->backing_format) ? > + backing_hd->drv ? backing_hd->drv->format_name : ""); > + bdrv_ref(bs->backing_hd); > +out: > + bdrv_refresh_limits(bs); > +} > + > /* > * Opens the backing file for a BlockDriverState if not yet open > * > @@ -1085,6 +1105,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) > char backing_filename[PATH_MAX]; > int back_flags, ret; > BlockDriver *back_drv = NULL; > + BlockDriverState *backing_hd; > Error *local_err = NULL; > > if (bs->backing_hd != NULL) { > @@ -1108,7 +1129,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) > sizeof(backing_filename)); > } > > - bs->backing_hd = bdrv_new(""); > + backing_hd = bdrv_new(""); > > if (bs->backing_format[0] != '\0') { > back_drv = bdrv_find_format(bs->backing_format); > @@ -1118,23 +1139,18 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) > back_flags = bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | > BDRV_O_COPY_ON_READ); > > - ret = bdrv_open(bs->backing_hd, > + ret = bdrv_open(backing_hd, > *backing_filename ? backing_filename : NULL, options, > back_flags, back_drv, &local_err); > if (ret < 0) { > - bdrv_unref(bs->backing_hd); > - bs->backing_hd = NULL; > + bdrv_unref(backing_hd); Here I wonder if this way of calling bdrv_open and doing a bdrv_unref confict with Max latests bdrv_open patches. You probably need to rebase and get rid of this bdrv_unref() : check Max series to be sure. > bs->open_flags |= BDRV_O_NO_BACKING; > error_setg(errp, "Could not open backing file: %s", > error_get_pretty(local_err)); > error_free(local_err); > return ret; > } > - > - if (bs->backing_hd->file) { > - pstrcpy(bs->backing_file, sizeof(bs->backing_file), > - bs->backing_hd->file->filename); > - } > + bdrv_set_backing_hd(bs, backing_hd); > > /* Recalculate the BlockLimits with the backing file */ > bdrv_refresh_limits(bs); > diff --git a/include/block/block.h b/include/block/block.h > index 9125bbe..f449753 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -208,6 +208,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, > int bdrv_open_image(BlockDriverState **pbs, const char *filename, > QDict *options, const char *bdref_key, int flags, > bool force_raw, bool allow_none, Error **errp); > +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd); > int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp); > int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, > int flags, BlockDriver *drv, Error **errp); > -- > 1.8.5.3 > > bdrv_set_backing_hd seems to be handy since some people want to dynamically change backing_file using QMP to move them from slow storage to fast storage and the reverse. Best regards Benoît
On Thu, 02/13 13:49, Benoît Canet wrote: > The Wednesday 29 Jan 2014 à 13:07:32 (+0800), Fam Zheng wrote : > > This is the common but non-trivial steps to assign or change the > > backing_hd of BDS. > > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > block.c | 34 +++++++++++++++++++++++++--------- > > include/block/block.h | 1 + > > 2 files changed, 26 insertions(+), 9 deletions(-) > > > > diff --git a/block.c b/block.c > > index 8000ac0..c4eaa37 100644 > > --- a/block.c > > +++ b/block.c > > @@ -1072,6 +1072,26 @@ fail: > > return ret; > > } > > > > +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd) > > +{ > > + if (bs->backing_hd) { > > + bdrv_unref(bs->backing_hd); > > + } > > + > > + bs->backing_hd = backing_hd; > > + if (!backing_hd) { > > + bs->backing_file[0] = '\0'; > > + bs->backing_format[0] = '\0'; > > + goto out; > > + } > > + pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename); > > > > + pstrcpy(bs->backing_format, sizeof(bs->backing_file), > > I don't understand the sizeof(bs->backing_file). Is it sizeof(bs->backing_format) ? > Yes. > > + backing_hd->drv ? backing_hd->drv->format_name : ""); > > + bdrv_ref(bs->backing_hd); > > +out: > > + bdrv_refresh_limits(bs); > > +} > > + > > /* > > * Opens the backing file for a BlockDriverState if not yet open > > * > > @@ -1085,6 +1105,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) > > char backing_filename[PATH_MAX]; > > int back_flags, ret; > > BlockDriver *back_drv = NULL; > > + BlockDriverState *backing_hd; > > Error *local_err = NULL; > > > > if (bs->backing_hd != NULL) { > > @@ -1108,7 +1129,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) > > sizeof(backing_filename)); > > } > > > > - bs->backing_hd = bdrv_new(""); > > + backing_hd = bdrv_new(""); > > > > if (bs->backing_format[0] != '\0') { > > back_drv = bdrv_find_format(bs->backing_format); > > @@ -1118,23 +1139,18 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) > > back_flags = bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | > > BDRV_O_COPY_ON_READ); > > > > - ret = bdrv_open(bs->backing_hd, > > + ret = bdrv_open(backing_hd, > > *backing_filename ? backing_filename : NULL, options, > > back_flags, back_drv, &local_err); > > if (ret < 0) { > > - bdrv_unref(bs->backing_hd); > > - bs->backing_hd = NULL; > > + bdrv_unref(backing_hd); > Here I wonder if this way of calling bdrv_open and doing a bdrv_unref confict > with Max latests bdrv_open patches. > You probably need to rebase and get rid of this bdrv_unref() : check Max series > to be sure. I'll check it. > > > bs->open_flags |= BDRV_O_NO_BACKING; > > error_setg(errp, "Could not open backing file: %s", > > error_get_pretty(local_err)); > > error_free(local_err); > > return ret; > > } > > - > > - if (bs->backing_hd->file) { > > - pstrcpy(bs->backing_file, sizeof(bs->backing_file), > > - bs->backing_hd->file->filename); > > - } > > + bdrv_set_backing_hd(bs, backing_hd); > > > > /* Recalculate the BlockLimits with the backing file */ > > bdrv_refresh_limits(bs); > > diff --git a/include/block/block.h b/include/block/block.h > > index 9125bbe..f449753 100644 > > --- a/include/block/block.h > > +++ b/include/block/block.h > > @@ -208,6 +208,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, > > int bdrv_open_image(BlockDriverState **pbs, const char *filename, > > QDict *options, const char *bdref_key, int flags, > > bool force_raw, bool allow_none, Error **errp); > > +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd); > > int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp); > > int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, > > int flags, BlockDriver *drv, Error **errp); > > -- > > 1.8.5.3 > > > > > > bdrv_set_backing_hd seems to be handy since some people want to dynamically > change backing_file using QMP to move them from slow storage to fast storage and > the reverse. > Thanks, Fam
diff --git a/block.c b/block.c index 8000ac0..c4eaa37 100644 --- a/block.c +++ b/block.c @@ -1072,6 +1072,26 @@ fail: return ret; } +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd) +{ + if (bs->backing_hd) { + bdrv_unref(bs->backing_hd); + } + + bs->backing_hd = backing_hd; + if (!backing_hd) { + bs->backing_file[0] = '\0'; + bs->backing_format[0] = '\0'; + goto out; + } + pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename); + pstrcpy(bs->backing_format, sizeof(bs->backing_file), + backing_hd->drv ? backing_hd->drv->format_name : ""); + bdrv_ref(bs->backing_hd); +out: + bdrv_refresh_limits(bs); +} + /* * Opens the backing file for a BlockDriverState if not yet open * @@ -1085,6 +1105,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) char backing_filename[PATH_MAX]; int back_flags, ret; BlockDriver *back_drv = NULL; + BlockDriverState *backing_hd; Error *local_err = NULL; if (bs->backing_hd != NULL) { @@ -1108,7 +1129,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) sizeof(backing_filename)); } - bs->backing_hd = bdrv_new(""); + backing_hd = bdrv_new(""); if (bs->backing_format[0] != '\0') { back_drv = bdrv_find_format(bs->backing_format); @@ -1118,23 +1139,18 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) back_flags = bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_COPY_ON_READ); - ret = bdrv_open(bs->backing_hd, + ret = bdrv_open(backing_hd, *backing_filename ? backing_filename : NULL, options, back_flags, back_drv, &local_err); if (ret < 0) { - bdrv_unref(bs->backing_hd); - bs->backing_hd = NULL; + bdrv_unref(backing_hd); bs->open_flags |= BDRV_O_NO_BACKING; error_setg(errp, "Could not open backing file: %s", error_get_pretty(local_err)); error_free(local_err); return ret; } - - if (bs->backing_hd->file) { - pstrcpy(bs->backing_file, sizeof(bs->backing_file), - bs->backing_hd->file->filename); - } + bdrv_set_backing_hd(bs, backing_hd); /* Recalculate the BlockLimits with the backing file */ bdrv_refresh_limits(bs); diff --git a/include/block/block.h b/include/block/block.h index 9125bbe..f449753 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -208,6 +208,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, int bdrv_open_image(BlockDriverState **pbs, const char *filename, QDict *options, const char *bdref_key, int flags, bool force_raw, bool allow_none, Error **errp); +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd); int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp); int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, int flags, BlockDriver *drv, Error **errp);
This is the common but non-trivial steps to assign or change the backing_hd of BDS. Signed-off-by: Fam Zheng <famz@redhat.com> --- block.c | 34 +++++++++++++++++++++++++--------- include/block/block.h | 1 + 2 files changed, 26 insertions(+), 9 deletions(-)