| Message ID | 1379067909-22984-9-git-send-email-pl@kamp.de |
|---|---|
| State | New |
| Headers | show |
Il 13/09/2013 12:25, Peter Lieven ha scritto: > if the target has_zero_init = 0, but supports efficiently > writing zeroes by unmapping we call bdrv_zeroize to > avoid fully allocating the target. this currently > is designed especially for iscsi. > > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > qemu-img.c | 22 ++++++++++++++++------ > 1 file changed, 16 insertions(+), 6 deletions(-) > > diff --git a/qemu-img.c b/qemu-img.c > index 3e5e388..6eaddc6 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -1354,7 +1354,8 @@ static int img_convert(int argc, char **argv) > } > } > > - flags = BDRV_O_RDWR; > + flags = BDRV_O_RDWR | BDRV_O_UNMAP; I think this should be a new command-line flag. Paolo > + > ret = bdrv_parse_cache_flags(cache, &flags); > if (ret < 0) { > error_report("Invalid cache option: %s", cache); > @@ -1386,12 +1387,13 @@ static int img_convert(int argc, char **argv) > } > } > > + ret = bdrv_get_info(out_bs, &bdi); > + if (ret < 0) { > + error_report("could not get block driver info"); > + goto out; > + } > + > if (compress) { > - ret = bdrv_get_info(out_bs, &bdi); > - if (ret < 0) { > - error_report("could not get block driver info"); > - goto out; > - } > cluster_size = bdi.cluster_size; > if (cluster_size <= 0 || cluster_size > IO_BUF_SIZE) { > error_report("invalid cluster size"); > @@ -1470,6 +1472,14 @@ static int img_convert(int argc, char **argv) > } else { > int has_zero_init = bdrv_has_zero_init(out_bs); > > + if (!has_zero_init && !out_baseimg && bdi.discard_write_zeroes) { > + ret = bdrv_zeroize(out_bs, BDRV_REQ_MAY_UNMAP); > + if (ret < 0) { > + goto out; > + } > + has_zero_init = 1; > + } > + > sector_num = 0; // total number of sectors converted so far > nb_sectors = total_sectors - sector_num; > if (nb_sectors != 0) { >
On 13.09.2013 12:36, Paolo Bonzini wrote: > Il 13/09/2013 12:25, Peter Lieven ha scritto: >> if the target has_zero_init = 0, but supports efficiently >> writing zeroes by unmapping we call bdrv_zeroize to >> avoid fully allocating the target. this currently >> is designed especially for iscsi. >> >> Signed-off-by: Peter Lieven <pl@kamp.de> >> --- >> qemu-img.c | 22 ++++++++++++++++------ >> 1 file changed, 16 insertions(+), 6 deletions(-) >> >> diff --git a/qemu-img.c b/qemu-img.c >> index 3e5e388..6eaddc6 100644 >> --- a/qemu-img.c >> +++ b/qemu-img.c >> @@ -1354,7 +1354,8 @@ static int img_convert(int argc, char **argv) >> } >> } >> >> - flags = BDRV_O_RDWR; >> + flags = BDRV_O_RDWR | BDRV_O_UNMAP; > I think this should be a new command-line flag. In an earlier version there where no objections. I think it would make the usage of qemu-img convert more complicated. For most targets has_zero_init is 1 anyway. Peter
On 09/13/2013 04:36 AM, Paolo Bonzini wrote: > Il 13/09/2013 12:25, Peter Lieven ha scritto: >> if the target has_zero_init = 0, but supports efficiently >> writing zeroes by unmapping we call bdrv_zeroize to >> avoid fully allocating the target. this currently >> is designed especially for iscsi. >> >> Signed-off-by: Peter Lieven <pl@kamp.de> >> --- >> qemu-img.c | 22 ++++++++++++++++------ >> 1 file changed, 16 insertions(+), 6 deletions(-) >> >> diff --git a/qemu-img.c b/qemu-img.c >> index 3e5e388..6eaddc6 100644 >> --- a/qemu-img.c >> +++ b/qemu-img.c >> @@ -1354,7 +1354,8 @@ static int img_convert(int argc, char **argv) >> } >> } >> >> - flags = BDRV_O_RDWR; >> + flags = BDRV_O_RDWR | BDRV_O_UNMAP; > > I think this should be a new command-line flag. I agree - while 'sparse by default' may be reasonable, it is also feasible to want a mode that guarantees expansion rather than unmapped or sparse.
Am 13.09.2013 20:25, schrieb Eric Blake: > On 09/13/2013 04:36 AM, Paolo Bonzini wrote: >> Il 13/09/2013 12:25, Peter Lieven ha scritto: >>> if the target has_zero_init = 0, but supports efficiently >>> writing zeroes by unmapping we call bdrv_zeroize to >>> avoid fully allocating the target. this currently >>> is designed especially for iscsi. >>> >>> Signed-off-by: Peter Lieven <pl@kamp.de> >>> --- >>> qemu-img.c | 22 ++++++++++++++++------ >>> 1 file changed, 16 insertions(+), 6 deletions(-) >>> >>> diff --git a/qemu-img.c b/qemu-img.c >>> index 3e5e388..6eaddc6 100644 >>> --- a/qemu-img.c >>> +++ b/qemu-img.c >>> @@ -1354,7 +1354,8 @@ static int img_convert(int argc, char **argv) >>> } >>> } >>> >>> - flags = BDRV_O_RDWR; >>> + flags = BDRV_O_RDWR | BDRV_O_UNMAP; >> I think this should be a new command-line flag. > I agree - while 'sparse by default' may be reasonable, it is also > feasible to want a mode that guarantees expansion rather than unmapped > or sparse. > Ok, so do you find the proposed -S 0 bei Paolo a good choice? If this is supplied I would go as far as completly setting has_zero_init = 0 also for targets which default to 1. This would guaranteed exspansion and full allocation for all drivers. Peter
On 09/13/2013 01:48 PM, Peter Lieven wrote: >>>> + flags = BDRV_O_RDWR | BDRV_O_UNMAP; >>> I think this should be a new command-line flag. >> I agree - while 'sparse by default' may be reasonable, it is also >> feasible to want a mode that guarantees expansion rather than unmapped >> or sparse. >> > Ok, so do you find the proposed -S 0 bei Paolo a good choice? > If this is supplied I would go as far as completly setting > has_zero_init = 0 also for targets which default to 1. This > would guaranteed exspansion and full allocation for all drivers. Yeah, works for me - an optional mode that guarantees full allocation.
Il 13/09/2013 21:48, Peter Lieven ha scritto: > Am 13.09.2013 20:25, schrieb Eric Blake: >> On 09/13/2013 04:36 AM, Paolo Bonzini wrote: >>> Il 13/09/2013 12:25, Peter Lieven ha scritto: >>>> if the target has_zero_init = 0, but supports efficiently >>>> writing zeroes by unmapping we call bdrv_zeroize to >>>> avoid fully allocating the target. this currently >>>> is designed especially for iscsi. >>>> >>>> Signed-off-by: Peter Lieven <pl@kamp.de> >>>> --- >>>> qemu-img.c | 22 ++++++++++++++++------ >>>> 1 file changed, 16 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/qemu-img.c b/qemu-img.c >>>> index 3e5e388..6eaddc6 100644 >>>> --- a/qemu-img.c >>>> +++ b/qemu-img.c >>>> @@ -1354,7 +1354,8 @@ static int img_convert(int argc, char **argv) >>>> } >>>> } >>>> >>>> - flags = BDRV_O_RDWR; >>>> + flags = BDRV_O_RDWR | BDRV_O_UNMAP; >>> I think this should be a new command-line flag. >> I agree - while 'sparse by default' may be reasonable, it is also >> feasible to want a mode that guarantees expansion rather than unmapped >> or sparse. >> > Ok, so do you find the proposed -S 0 bei Paolo a good choice? > If this is supplied I would go as far as completly setting > has_zero_init = 0 also for targets which default to 1. This > would guaranteed exspansion and full allocation for all drivers. Sounds good. Paolo
diff --git a/qemu-img.c b/qemu-img.c index 3e5e388..6eaddc6 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1354,7 +1354,8 @@ static int img_convert(int argc, char **argv) } } - flags = BDRV_O_RDWR; + flags = BDRV_O_RDWR | BDRV_O_UNMAP; + ret = bdrv_parse_cache_flags(cache, &flags); if (ret < 0) { error_report("Invalid cache option: %s", cache); @@ -1386,12 +1387,13 @@ static int img_convert(int argc, char **argv) } } + ret = bdrv_get_info(out_bs, &bdi); + if (ret < 0) { + error_report("could not get block driver info"); + goto out; + } + if (compress) { - ret = bdrv_get_info(out_bs, &bdi); - if (ret < 0) { - error_report("could not get block driver info"); - goto out; - } cluster_size = bdi.cluster_size; if (cluster_size <= 0 || cluster_size > IO_BUF_SIZE) { error_report("invalid cluster size"); @@ -1470,6 +1472,14 @@ static int img_convert(int argc, char **argv) } else { int has_zero_init = bdrv_has_zero_init(out_bs); + if (!has_zero_init && !out_baseimg && bdi.discard_write_zeroes) { + ret = bdrv_zeroize(out_bs, BDRV_REQ_MAY_UNMAP); + if (ret < 0) { + goto out; + } + has_zero_init = 1; + } + sector_num = 0; // total number of sectors converted so far nb_sectors = total_sectors - sector_num; if (nb_sectors != 0) {
if the target has_zero_init = 0, but supports efficiently writing zeroes by unmapping we call bdrv_zeroize to avoid fully allocating the target. this currently is designed especially for iscsi. Signed-off-by: Peter Lieven <pl@kamp.de> --- qemu-img.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)