| Message ID | 1371934712-11714-7-git-send-email-pl@kamp.de |
|---|---|
| State | New |
| Headers | show |
Il 22/06/2013 22:58, Peter Lieven ha scritto: > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > qemu-img.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/qemu-img.c b/qemu-img.c > index 809b4f1..5aa53ab 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -1513,9 +1513,13 @@ static int img_convert(int argc, char **argv) > If the output is to a host device, we also write out > sectors that are entirely 0, since whatever data was > already there is garbage, not 0s. */ > - if (!has_zero_init || out_baseimg || > - is_allocated_sectors_min(buf1, n, &n1, min_sparse)) { > - ret = bdrv_write(out_bs, sector_num, buf1, n1); > + int allocated = is_allocated_sectors_min(buf1, n, &n1, min_sparse); > + if (!has_zero_init || out_baseimg || allocated) { > + if (allocated || out_baseimg) { > + ret = bdrv_write(out_bs, sector_num, buf1, n1); > + } else { > + ret = bdrv_write_zeroes(out_bs, sector_num, n1); I think it should still do the write only if !has_zero_init. Paolo > + } > if (ret < 0) { > error_report("error while writing sector %" PRId64 > ": %s", sector_num, strerror(-ret)); >
Am 24.06.2013 16:33, schrieb Paolo Bonzini: > Il 22/06/2013 22:58, Peter Lieven ha scritto: >> Signed-off-by: Peter Lieven <pl@kamp.de> >> --- >> qemu-img.c | 10 +++++++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/qemu-img.c b/qemu-img.c >> index 809b4f1..5aa53ab 100644 >> --- a/qemu-img.c >> +++ b/qemu-img.c >> @@ -1513,9 +1513,13 @@ static int img_convert(int argc, char **argv) >> If the output is to a host device, we also write out >> sectors that are entirely 0, since whatever data was >> already there is garbage, not 0s. */ >> - if (!has_zero_init || out_baseimg || >> - is_allocated_sectors_min(buf1, n, &n1, min_sparse)) { >> - ret = bdrv_write(out_bs, sector_num, buf1, n1); >> + int allocated = is_allocated_sectors_min(buf1, n, &n1, min_sparse); >> + if (!has_zero_init || out_baseimg || allocated) { >> + if (allocated || out_baseimg) { >> + ret = bdrv_write(out_bs, sector_num, buf1, n1); >> + } else { >> + ret = bdrv_write_zeroes(out_bs, sector_num, n1); > I think it should still do the write only if !has_zero_init. With this I am still in trouble with iSCSI, but Kevin pointed out another possible solution for the allocating everything problem. a) let iscsi_create discard the whole device if lbpz && lbprz b) return 1 for has_zero_init if lbprz. What do you think? Peter > > Paolo > >> + } >> if (ret < 0) { >> error_report("error while writing sector %" PRId64 >> ": %s", sector_num, strerror(-ret)); >>
Il 24/06/2013 18:17, Peter Lieven ha scritto: > Am 24.06.2013 16:33, schrieb Paolo Bonzini: >> Il 22/06/2013 22:58, Peter Lieven ha scritto: >>> Signed-off-by: Peter Lieven <pl@kamp.de> >>> --- >>> qemu-img.c | 10 +++++++--- >>> 1 file changed, 7 insertions(+), 3 deletions(-) >>> >>> diff --git a/qemu-img.c b/qemu-img.c >>> index 809b4f1..5aa53ab 100644 >>> --- a/qemu-img.c >>> +++ b/qemu-img.c >>> @@ -1513,9 +1513,13 @@ static int img_convert(int argc, char **argv) >>> If the output is to a host device, we also write out >>> sectors that are entirely 0, since whatever data was >>> already there is garbage, not 0s. */ >>> - if (!has_zero_init || out_baseimg || >>> - is_allocated_sectors_min(buf1, n, &n1, min_sparse)) { >>> - ret = bdrv_write(out_bs, sector_num, buf1, n1); >>> + int allocated = is_allocated_sectors_min(buf1, n, &n1, min_sparse); >>> + if (!has_zero_init || out_baseimg || allocated) { >>> + if (allocated || out_baseimg) { >>> + ret = bdrv_write(out_bs, sector_num, buf1, n1); >>> + } else { >>> + ret = bdrv_write_zeroes(out_bs, sector_num, n1); >> I think it should still do the write only if !has_zero_init. > With this I am still in trouble with iSCSI, but Kevin pointed out another > possible solution for the allocating everything problem. > > a) let iscsi_create discard the whole device if lbpz && lbprz > b) return 1 for has_zero_init if lbprz. > > What do you think? Fine by me (but the discard may take a looong time! :)). Paolo
Am 24.06.2013 18:25, schrieb Paolo Bonzini: > Il 24/06/2013 18:17, Peter Lieven ha scritto: >> Am 24.06.2013 16:33, schrieb Paolo Bonzini: >>> Il 22/06/2013 22:58, Peter Lieven ha scritto: >>>> Signed-off-by: Peter Lieven <pl@kamp.de> >>>> --- >>>> qemu-img.c | 10 +++++++--- >>>> 1 file changed, 7 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/qemu-img.c b/qemu-img.c >>>> index 809b4f1..5aa53ab 100644 >>>> --- a/qemu-img.c >>>> +++ b/qemu-img.c >>>> @@ -1513,9 +1513,13 @@ static int img_convert(int argc, char **argv) >>>> If the output is to a host device, we also write out >>>> sectors that are entirely 0, since whatever data was >>>> already there is garbage, not 0s. */ >>>> - if (!has_zero_init || out_baseimg || >>>> - is_allocated_sectors_min(buf1, n, &n1, min_sparse)) { >>>> - ret = bdrv_write(out_bs, sector_num, buf1, n1); >>>> + int allocated = is_allocated_sectors_min(buf1, n, &n1, min_sparse); >>>> + if (!has_zero_init || out_baseimg || allocated) { >>>> + if (allocated || out_baseimg) { >>>> + ret = bdrv_write(out_bs, sector_num, buf1, n1); >>>> + } else { >>>> + ret = bdrv_write_zeroes(out_bs, sector_num, n1); >>> I think it should still do the write only if !has_zero_init. >> With this I am still in trouble with iSCSI, but Kevin pointed out another >> possible solution for the allocating everything problem. >> >> a) let iscsi_create discard the whole device if lbpz && lbprz >> b) return 1 for has_zero_init if lbprz. >> >> What do you think? > Fine by me (but the discard may take a looong time! :)). This was also my concern, but if a target advertises logical block provisioning it should be able to discard blocks fast (and not overwrite all of them or sth like that). Peter > > Paolo >
Am 24.06.2013 18:25, schrieb Paolo Bonzini: > Il 24/06/2013 18:17, Peter Lieven ha scritto: >> Am 24.06.2013 16:33, schrieb Paolo Bonzini: >>> Il 22/06/2013 22:58, Peter Lieven ha scritto: >>>> Signed-off-by: Peter Lieven <pl@kamp.de> >>>> --- >>>> qemu-img.c | 10 +++++++--- >>>> 1 file changed, 7 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/qemu-img.c b/qemu-img.c >>>> index 809b4f1..5aa53ab 100644 >>>> --- a/qemu-img.c >>>> +++ b/qemu-img.c >>>> @@ -1513,9 +1513,13 @@ static int img_convert(int argc, char **argv) >>>> If the output is to a host device, we also write out >>>> sectors that are entirely 0, since whatever data was >>>> already there is garbage, not 0s. */ >>>> - if (!has_zero_init || out_baseimg || >>>> - is_allocated_sectors_min(buf1, n, &n1, min_sparse)) { >>>> - ret = bdrv_write(out_bs, sector_num, buf1, n1); >>>> + int allocated = is_allocated_sectors_min(buf1, n, &n1, min_sparse); >>>> + if (!has_zero_init || out_baseimg || allocated) { >>>> + if (allocated || out_baseimg) { >>>> + ret = bdrv_write(out_bs, sector_num, buf1, n1); >>>> + } else { >>>> + ret = bdrv_write_zeroes(out_bs, sector_num, n1); >>> I think it should still do the write only if !has_zero_init. >> With this I am still in trouble with iSCSI, but Kevin pointed out another >> possible solution for the allocating everything problem. >> >> a) let iscsi_create discard the whole device if lbpz && lbprz >> b) return 1 for has_zero_init if lbprz. >> >> What do you think? > Fine by me (but the discard may take a looong time! :)). fyi: i have created an optimized unmap function which transmits the maximum allowed unmap_list length (read from block_limits page). for 1TB it takes approx. 1 minute on my storage. i will include this in v2 of the series and also add the optimization to the other unmap calls (which currently do not check max_unmap size at all). Peter
diff --git a/qemu-img.c b/qemu-img.c index 809b4f1..5aa53ab 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1513,9 +1513,13 @@ static int img_convert(int argc, char **argv) If the output is to a host device, we also write out sectors that are entirely 0, since whatever data was already there is garbage, not 0s. */ - if (!has_zero_init || out_baseimg || - is_allocated_sectors_min(buf1, n, &n1, min_sparse)) { - ret = bdrv_write(out_bs, sector_num, buf1, n1); + int allocated = is_allocated_sectors_min(buf1, n, &n1, min_sparse); + if (!has_zero_init || out_baseimg || allocated) { + if (allocated || out_baseimg) { + ret = bdrv_write(out_bs, sector_num, buf1, n1); + } else { + ret = bdrv_write_zeroes(out_bs, sector_num, n1); + } if (ret < 0) { error_report("error while writing sector %" PRId64 ": %s", sector_num, strerror(-ret));
Signed-off-by: Peter Lieven <pl@kamp.de> --- qemu-img.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)