| Message ID | 20200204170848.614480-2-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | block: Introduce real BdrvChildRole | expand |
On 2/4/20 11:08 AM, Max Reitz wrote: > We want to unify child_format and child_file at some point. One of the > important things that set format drivers apart from other drivers is > that they do not expect other format nodes under them (except in the > backing chain). That means we need something on which to distinguish > format drivers from others, and hence this flag. It _is_ possible to set up a format node on top of another; in fact, our testsuite does that in at least iotest 072. I agree that setups like 'qcow2 - qcow2 - file' are uncommon, but the setup 'qcow2 - raw - file' may be useful for extracting a partition of a raw disk when it is known that the partition in that disk itself contains qcow2 data. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > +++ b/include/block/block_int.h > @@ -94,6 +94,13 @@ struct BlockDriver { > * must implement them and return -ENOTSUP. > */ > bool is_filter; > + /* > + * Set to true if the BlockDriver is a format driver. Format nodes > + * generally do not expect their children to be other format nodes > + * (except for backing files), and so format probing is disabled > + * on those children. Aha - nested formats ARE still allowed when you explicitly request it (which is what iotest 72 does) - what you are stating here is that implicit probing of is forbidden for a parent declared as a format driver. That makes more sense. I'm not sure if the commit message needs a tweak, but the patch itself is sane as-is. Reviewed-by: Eric Blake <eblake@redhat.com> > + */ > + bool is_format; > /* > * Return true if @to_replace can be replaced by a BDS with the > * same data as @bs without it affecting @bs's behavior (that is, >
On 05.02.20 14:51, Eric Blake wrote: > On 2/4/20 11:08 AM, Max Reitz wrote: >> We want to unify child_format and child_file at some point. One of the >> important things that set format drivers apart from other drivers is >> that they do not expect other format nodes under them (except in the >> backing chain). That means we need something on which to distinguish >> format drivers from others, and hence this flag. > > It _is_ possible to set up a format node on top of another; Sure, but format nodes don’t “expect” that. That is, it isn’t what we should strive to do by default. (Trying to do format probing inside of an already probed format would be a security risk, actually.) > in fact, our > testsuite does that in at least iotest 072. I agree that setups like > 'qcow2 - qcow2 - file' are uncommon, but the setup 'qcow2 - raw - file' > may be useful for extracting a partition of a raw disk when it is known > that the partition in that disk itself contains qcow2 data. > >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- > >> +++ b/include/block/block_int.h >> @@ -94,6 +94,13 @@ struct BlockDriver { >> * must implement them and return -ENOTSUP. >> */ >> bool is_filter; >> + /* >> + * Set to true if the BlockDriver is a format driver. Format nodes >> + * generally do not expect their children to be other format nodes >> + * (except for backing files), and so format probing is disabled >> + * on those children. > > Aha - nested formats ARE still allowed when you explicitly request it > (which is what iotest 72 does) - what you are stating here is that > implicit probing of is forbidden for a parent declared as a format > driver. That makes more sense. > > I'm not sure if the commit message needs a tweak, but the patch itself > is sane as-is. I’m not sure either. I think “expect” is OK as-is. I suppose I could add a sentence like “... they do not expect other format nodes under them (...). That is, we must not probe formats inside of formats.” > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks for reviewing! Max
On Tue 04 Feb 2020 06:08:16 PM CET, Max Reitz wrote: > We want to unify child_format and child_file at some point. One of the > important things that set format drivers apart from other drivers is > that they do not expect other format nodes under them (except in the > backing chain). That means we need something on which to distinguish > format drivers from others, and hence this flag. > > Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Berto
diff --git a/block/bochs.c b/block/bochs.c index 32bb83b268..e7bbeaa1c4 100644 --- a/block/bochs.c +++ b/block/bochs.c @@ -301,6 +301,7 @@ static BlockDriver bdrv_bochs = { .bdrv_refresh_limits = bochs_refresh_limits, .bdrv_co_preadv = bochs_co_preadv, .bdrv_close = bochs_close, + .is_format = true, }; static void bdrv_bochs_init(void) diff --git a/block/cloop.c b/block/cloop.c index 4de94876d4..f90f1a4b4c 100644 --- a/block/cloop.c +++ b/block/cloop.c @@ -297,6 +297,7 @@ static BlockDriver bdrv_cloop = { .bdrv_refresh_limits = cloop_refresh_limits, .bdrv_co_preadv = cloop_co_preadv, .bdrv_close = cloop_close, + .is_format = true, }; static void bdrv_cloop_init(void) diff --git a/block/crypto.c b/block/crypto.c index 24823835c1..b81f673a51 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -673,6 +673,8 @@ static BlockDriver bdrv_crypto_luks = { .bdrv_get_info = block_crypto_get_info_luks, .bdrv_get_specific_info = block_crypto_get_specific_info_luks, + .is_format = true, + .strong_runtime_opts = block_crypto_strong_runtime_opts, }; diff --git a/block/dmg.c b/block/dmg.c index 4a045f2b3e..ef3c6e771d 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -753,6 +753,7 @@ static BlockDriver bdrv_dmg = { .bdrv_child_perm = bdrv_format_default_perms, .bdrv_co_preadv = dmg_co_preadv, .bdrv_close = dmg_close, + .is_format = true, }; static void bdrv_dmg_init(void) diff --git a/block/parallels.c b/block/parallels.c index 7a01997659..30eda982bd 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -910,6 +910,7 @@ static BlockDriver bdrv_parallels = { .bdrv_co_flush_to_os = parallels_co_flush_to_os, .bdrv_co_readv = parallels_co_readv, .bdrv_co_writev = parallels_co_writev, + .is_format = true, .supports_backing = true, .bdrv_co_create = parallels_co_create, .bdrv_co_create_opts = parallels_co_create_opts, diff --git a/block/qcow.c b/block/qcow.c index fce8989868..0e4f09934c 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -1184,6 +1184,7 @@ static BlockDriver bdrv_qcow = { .bdrv_co_create = qcow_co_create, .bdrv_co_create_opts = qcow_co_create_opts, .bdrv_has_zero_init = bdrv_has_zero_init_1, + .is_format = true, .supports_backing = true, .bdrv_refresh_limits = qcow_refresh_limits, diff --git a/block/qcow2.c b/block/qcow2.c index ef96606f8d..8e057556eb 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -5566,6 +5566,7 @@ BlockDriver bdrv_qcow2 = { .bdrv_save_vmstate = qcow2_save_vmstate, .bdrv_load_vmstate = qcow2_load_vmstate, + .is_format = true, .supports_backing = true, .bdrv_change_backing_file = qcow2_change_backing_file, diff --git a/block/qed.c b/block/qed.c index d8c4e5fb1e..eb6139acbc 100644 --- a/block/qed.c +++ b/block/qed.c @@ -1663,6 +1663,7 @@ static BlockDriver bdrv_qed = { .format_name = "qed", .instance_size = sizeof(BDRVQEDState), .create_opts = &qed_create_opts, + .is_format = true, .supports_backing = true, .bdrv_probe = bdrv_qed_probe, diff --git a/block/raw-format.c b/block/raw-format.c index 3a76ec7dd2..2429ff0d23 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -567,6 +567,7 @@ BlockDriver bdrv_raw = { .bdrv_co_copy_range_to = &raw_co_copy_range_to, .bdrv_co_truncate = &raw_co_truncate, .bdrv_getlength = &raw_getlength, + .is_format = true, .has_variable_length = true, .bdrv_measure = &raw_measure, .bdrv_get_info = &raw_get_info, diff --git a/block/vdi.c b/block/vdi.c index 0142da7233..7e87b205b5 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -1051,6 +1051,7 @@ static BlockDriver bdrv_vdi = { .bdrv_get_info = vdi_get_info, + .is_format = true, .create_opts = &vdi_create_opts, .bdrv_co_check = vdi_co_check, }; diff --git a/block/vhdx.c b/block/vhdx.c index f02d2611be..d3c1619026 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -2241,6 +2241,7 @@ static BlockDriver bdrv_vhdx = { .bdrv_co_check = vhdx_co_check, .bdrv_has_zero_init = vhdx_has_zero_init, + .is_format = true, .create_opts = &vhdx_create_opts, }; diff --git a/block/vmdk.c b/block/vmdk.c index 20e909d997..631fcd15ab 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -3058,6 +3058,7 @@ static BlockDriver bdrv_vmdk = { .bdrv_get_info = vmdk_get_info, .bdrv_gather_child_options = vmdk_gather_child_options, + .is_format = true, .supports_backing = true, .create_opts = &vmdk_create_opts, }; diff --git a/block/vpc.c b/block/vpc.c index a65550298e..7cf0f87a16 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -1248,6 +1248,7 @@ static BlockDriver bdrv_vpc = { .bdrv_get_info = vpc_get_info, + .is_format = true, .create_opts = &vpc_create_opts, .bdrv_has_zero_init = vpc_has_zero_init, .strong_runtime_opts = vpc_strong_runtime_opts, diff --git a/include/block/block_int.h b/include/block/block_int.h index 589a797fab..7fb40281e4 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -94,6 +94,13 @@ struct BlockDriver { * must implement them and return -ENOTSUP. */ bool is_filter; + /* + * Set to true if the BlockDriver is a format driver. Format nodes + * generally do not expect their children to be other format nodes + * (except for backing files), and so format probing is disabled + * on those children. + */ + bool is_format; /* * Return true if @to_replace can be replaced by a BDS with the * same data as @bs without it affecting @bs's behavior (that is,
We want to unify child_format and child_file at some point. One of the important things that set format drivers apart from other drivers is that they do not expect other format nodes under them (except in the backing chain). That means we need something on which to distinguish format drivers from others, and hence this flag. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/bochs.c | 1 + block/cloop.c | 1 + block/crypto.c | 2 ++ block/dmg.c | 1 + block/parallels.c | 1 + block/qcow.c | 1 + block/qcow2.c | 1 + block/qed.c | 1 + block/raw-format.c | 1 + block/vdi.c | 1 + block/vhdx.c | 1 + block/vmdk.c | 1 + block/vpc.c | 1 + include/block/block_int.h | 7 +++++++ 14 files changed, 21 insertions(+)