| Message ID | 1551346019-293202-1-git-send-email-andrey.shinkevich@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] qcow2: discard bitmap when removed | expand |
28.02.2019 12:26, Andrey Shinkevich wrote: > When a bitmap is removed, we can clean some space on the disk. The size > of a cluster may be larger, so is the size of the bitmap that includes > many clusters. Some bitmaps can be as large as tens of megabytes. > The flag QCOW2_DISCARD_ALWAYS allows a call to the raw_co_pdiscard() > that does the actual cleaning of the image on disk, while with the flag > QCOW2_DISCARD_OTHER, a reference count of the cluster is updated only. not quite right, as it depends on configuration. So I'd reword the whole message to be simpler, like: Bitmap data may take a lot of disk space, so it's better to discard it always. > > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > v1: Discard old bitmap directories in QCOW2 image > > In the first version of the patch, any call to qcow2_free_clusters() in > block/qcow2-bitmap.c was with the flag QCOW2_DISCARD_ALWAYS, even in the > cases when the cleaned space is insignificant, particularly, in case of > bitmap directories. > Discussed in the email thread with the message ID > <1549974951-731285-1-git-send-email-andrey.shinkevich@virtuozzo.com> > > block/qcow2-bitmap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c > index 3ee524d..162744e 100644 > --- a/block/qcow2-bitmap.c > +++ b/block/qcow2-bitmap.c > @@ -202,7 +202,7 @@ static void clear_bitmap_table(BlockDriverState *bs, uint64_t *bitmap_table, > continue; > } > > - qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_OTHER); > + qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_ALWAYS); > bitmap_table[i] = 0; > } > } >
On 2/28/19 4:26 AM, Andrey Shinkevich wrote: > When a bitmap is removed, we can clean some space on the disk. The size > of a cluster may be larger, so is the size of the bitmap that includes > many clusters. Some bitmaps can be as large as tens of megabytes. > The flag QCOW2_DISCARD_ALWAYS allows a call to the raw_co_pdiscard() > that does the actual cleaning of the image on disk, while with the flag > QCOW2_DISCARD_OTHER, a reference count of the cluster is updated only. > > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> > --- > v1: Discard old bitmap directories in QCOW2 image > > In the first version of the patch, any call to qcow2_free_clusters() in > block/qcow2-bitmap.c was with the flag QCOW2_DISCARD_ALWAYS, even in the > cases when the cleaned space is insignificant, particularly, in case of > bitmap directories. > Discussed in the email thread with the message ID > <1549974951-731285-1-git-send-email-andrey.shinkevich@virtuozzo.com> > > block/qcow2-bitmap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c > index 3ee524d..162744e 100644 > --- a/block/qcow2-bitmap.c > +++ b/block/qcow2-bitmap.c > @@ -202,7 +202,7 @@ static void clear_bitmap_table(BlockDriverState *bs, uint64_t *bitmap_table, > continue; > } > > - qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_OTHER); > + qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_ALWAYS); > bitmap_table[i] = 0; > } > } > Looks like 4.1 material now. Kevin may have been expecting me to engage with this, even though I do not maintain the qcow2-bitmap.c file. (And I'm very behind on reading mail that was not directly addressed to me, sorry...!) Please resend when the 4.1 window opens and we'll address this then. --js
On 19/03/2019 01:18, John Snow wrote: > > > On 2/28/19 4:26 AM, Andrey Shinkevich wrote: >> When a bitmap is removed, we can clean some space on the disk. The size >> of a cluster may be larger, so is the size of the bitmap that includes >> many clusters. Some bitmaps can be as large as tens of megabytes. >> The flag QCOW2_DISCARD_ALWAYS allows a call to the raw_co_pdiscard() >> that does the actual cleaning of the image on disk, while with the flag >> QCOW2_DISCARD_OTHER, a reference count of the cluster is updated only. >> >> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >> --- >> v1: Discard old bitmap directories in QCOW2 image >> >> In the first version of the patch, any call to qcow2_free_clusters() in >> block/qcow2-bitmap.c was with the flag QCOW2_DISCARD_ALWAYS, even in the >> cases when the cleaned space is insignificant, particularly, in case of >> bitmap directories. >> Discussed in the email thread with the message ID >> <1549974951-731285-1-git-send-email-andrey.shinkevich@virtuozzo.com> >> >> block/qcow2-bitmap.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c >> index 3ee524d..162744e 100644 >> --- a/block/qcow2-bitmap.c >> +++ b/block/qcow2-bitmap.c >> @@ -202,7 +202,7 @@ static void clear_bitmap_table(BlockDriverState *bs, uint64_t *bitmap_table, >> continue; >> } >> >> - qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_OTHER); >> + qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_ALWAYS); >> bitmap_table[i] = 0; >> } >> } >> > Looks like 4.1 material now. Kevin may have been expecting me to engage > with this, even though I do not maintain the qcow2-bitmap.c file. (And > I'm very behind on reading mail that was not directly addressed to me, > sorry...!) > > Please resend when the 4.1 window opens and we'll address this then. > > --js > Thank you, John. Well noted. Andrey
PINGING I kindly remind you about this small patch for QEMU 4.1. I am going to be away for vacation from April, 10th to May, 15th.
On 28.02.19 10:26, Andrey Shinkevich wrote: > When a bitmap is removed, we can clean some space on the disk. The size > of a cluster may be larger, so is the size of the bitmap that includes > many clusters. Some bitmaps can be as large as tens of megabytes. > The flag QCOW2_DISCARD_ALWAYS allows a call to the raw_co_pdiscard() > that does the actual cleaning of the image on disk, while with the flag > QCOW2_DISCARD_OTHER, a reference count of the cluster is updated only. > > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> > --- > v1: Discard old bitmap directories in QCOW2 image > > In the first version of the patch, any call to qcow2_free_clusters() in > block/qcow2-bitmap.c was with the flag QCOW2_DISCARD_ALWAYS, even in the > cases when the cleaned space is insignificant, particularly, in case of > bitmap directories. > Discussed in the email thread with the message ID > <1549974951-731285-1-git-send-email-andrey.shinkevich@virtuozzo.com> > > block/qcow2-bitmap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Thanks, changed the commit message to Vladimir’s proposal and applied to my block-next branch: https://git.xanclic.moe/XanClic/qemu/commits/branch/block-next Max
On 08/04/2019 22:03, Max Reitz wrote: > On 28.02.19 10:26, Andrey Shinkevich wrote: >> When a bitmap is removed, we can clean some space on the disk. The size >> of a cluster may be larger, so is the size of the bitmap that includes >> many clusters. Some bitmaps can be as large as tens of megabytes. >> The flag QCOW2_DISCARD_ALWAYS allows a call to the raw_co_pdiscard() >> that does the actual cleaning of the image on disk, while with the flag >> QCOW2_DISCARD_OTHER, a reference count of the cluster is updated only. >> >> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >> --- >> v1: Discard old bitmap directories in QCOW2 image >> >> In the first version of the patch, any call to qcow2_free_clusters() in >> block/qcow2-bitmap.c was with the flag QCOW2_DISCARD_ALWAYS, even in the >> cases when the cleaned space is insignificant, particularly, in case of >> bitmap directories. >> Discussed in the email thread with the message ID >> <1549974951-731285-1-git-send-email-andrey.shinkevich@virtuozzo.com> >> >> block/qcow2-bitmap.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Thanks, changed the commit message to Vladimir’s proposal and applied to > my block-next branch: > > https://git.xanclic.moe/XanClic/qemu/commits/branch/block-next > > Max > Thank you, Max!
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index 3ee524d..162744e 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -202,7 +202,7 @@ static void clear_bitmap_table(BlockDriverState *bs, uint64_t *bitmap_table, continue; } - qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_OTHER); + qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_ALWAYS); bitmap_table[i] = 0; } }
When a bitmap is removed, we can clean some space on the disk. The size of a cluster may be larger, so is the size of the bitmap that includes many clusters. Some bitmaps can be as large as tens of megabytes. The flag QCOW2_DISCARD_ALWAYS allows a call to the raw_co_pdiscard() that does the actual cleaning of the image on disk, while with the flag QCOW2_DISCARD_OTHER, a reference count of the cluster is updated only. Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> --- v1: Discard old bitmap directories in QCOW2 image In the first version of the patch, any call to qcow2_free_clusters() in block/qcow2-bitmap.c was with the flag QCOW2_DISCARD_ALWAYS, even in the cases when the cleaned space is insignificant, particularly, in case of bitmap directories. Discussed in the email thread with the message ID <1549974951-731285-1-git-send-email-andrey.shinkevich@virtuozzo.com> block/qcow2-bitmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)