| Message ID | 1291130056-32441-4-git-send-email-stefanha@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Am 30.11.2010 16:14, schrieb Stefan Hajnoczi: > Backing filenames may contain a protocol. The code currently doesn't > consider this case and produces filenames that embed "<protocol>:". > Don't combine filenames if the backing filename contains a protocol. > > Based on an earlier patch by Anthony Liguori <aliguori@us.ibm.com>. > > Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> > --- > block.c | 15 +++++++++++---- > 1 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/block.c b/block.c > index 59b69e4..c9a6720 100644 > --- a/block.c > +++ b/block.c > @@ -611,10 +611,17 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, > BlockDriver *back_drv = NULL; > > bs->backing_hd = bdrv_new(""); > - path_combine(backing_filename, sizeof(backing_filename), > - filename, bs->backing_file); > - if (bs->backing_format[0] != '\0') > - back_drv = bdrv_find_format(bs->backing_format); > + if (path_has_protocol(bs->backing_file)) { > + back_drv = bdrv_find_protocol(bs->backing_file); > + pstrcpy(backing_filename, sizeof(backing_filename), > + bs->backing_file); > + } else { > + path_combine(backing_filename, sizeof(backing_filename), > + filename, bs->backing_file); > + if (bs->backing_format[0] != '\0') { > + back_drv = bdrv_find_format(bs->backing_format); > + } > + } Now we ignore bs->backing_format when a protocol is used. We don't even allow using anything on top of that protocol any more, so for example qcow2 over sheepdog would be broken. Maybe a use case that you like better would be QED over http or something. It might still not be a very common use case, but this doesn't look right to me. Probably only the pstrcpy/path_combine should be conditional. I've applied the first two patches of the series to the block branch. Kevin
On Thu, Dec 02, 2010 at 03:31:42PM +0100, Kevin Wolf wrote: > Am 30.11.2010 16:14, schrieb Stefan Hajnoczi: > > diff --git a/block.c b/block.c > > index 59b69e4..c9a6720 100644 > > --- a/block.c > > +++ b/block.c > > @@ -611,10 +611,17 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, > > BlockDriver *back_drv = NULL; > > > > bs->backing_hd = bdrv_new(""); > > - path_combine(backing_filename, sizeof(backing_filename), > > - filename, bs->backing_file); > > - if (bs->backing_format[0] != '\0') > > - back_drv = bdrv_find_format(bs->backing_format); > > + if (path_has_protocol(bs->backing_file)) { > > + back_drv = bdrv_find_protocol(bs->backing_file); > > + pstrcpy(backing_filename, sizeof(backing_filename), > > + bs->backing_file); > > + } else { > > + path_combine(backing_filename, sizeof(backing_filename), > > + filename, bs->backing_file); > > + if (bs->backing_format[0] != '\0') { > > + back_drv = bdrv_find_format(bs->backing_format); > > + } > > + } > > Now we ignore bs->backing_format when a protocol is used. We don't even > allow using anything on top of that protocol any more, so for example > qcow2 over sheepdog would be broken. Maybe a use case that you like > better would be QED over http or something. > > It might still not be a very common use case, but this doesn't look > right to me. Probably only the pstrcpy/path_combine should be conditional. True, I'll take a look at that. This reminds me of the write-up that Markus did on block driver trees and formats vs protocols. Stefan
diff --git a/block.c b/block.c index 59b69e4..c9a6720 100644 --- a/block.c +++ b/block.c @@ -611,10 +611,17 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, BlockDriver *back_drv = NULL; bs->backing_hd = bdrv_new(""); - path_combine(backing_filename, sizeof(backing_filename), - filename, bs->backing_file); - if (bs->backing_format[0] != '\0') - back_drv = bdrv_find_format(bs->backing_format); + if (path_has_protocol(bs->backing_file)) { + back_drv = bdrv_find_protocol(bs->backing_file); + pstrcpy(backing_filename, sizeof(backing_filename), + bs->backing_file); + } else { + path_combine(backing_filename, sizeof(backing_filename), + filename, bs->backing_file); + if (bs->backing_format[0] != '\0') { + back_drv = bdrv_find_format(bs->backing_format); + } + } /* backing files always opened read-only */ back_flags =
Backing filenames may contain a protocol. The code currently doesn't consider this case and produces filenames that embed "<protocol>:". Don't combine filenames if the backing filename contains a protocol. Based on an earlier patch by Anthony Liguori <aliguori@us.ibm.com>. Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> --- block.c | 15 +++++++++++---- 1 files changed, 11 insertions(+), 4 deletions(-)