Patchwork qed: make qed_alloc_clusters round up offset to nearest cluster

login
register
mail settings
Submitter Devin Nakamura
Date Aug. 15, 2011, 11:16 p.m.
Message ID <1313450170-22646-1-git-send-email-devin122@gmail.com>
Download mbox | patch
Permalink /patch/110108/
State New
Headers show

Comments

Devin Nakamura - Aug. 15, 2011, 11:16 p.m.
Signed-off-by: Devin Nakamura <devin122@gmail.com>
---
 block/qed.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Stefan Hajnoczi - Aug. 16, 2011, 4:22 p.m.
On Mon, Aug 15, 2011 at 07:16:10PM -0400, Devin Nakamura wrote:
> @@ -263,6 +263,8 @@ static int qed_read_string(BlockDriverState *file, uint64_t offset, size_t n,
>   */
>  static uint64_t qed_alloc_clusters(BDRVQEDState *s, unsigned int n)
>  {
> +    s->file_size =  qed_start_of_cluster(s, s->file_size +
> +                                         s->header.cluster_size - 1);

Why do you need this?  QED cluster aligns the file size on open.
file_size should always be cluster-aligned.

Stefan
Devin Nakamura - Aug. 16, 2011, 6:03 p.m.
On Tue, Aug 16, 2011 at 12:22 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Aug 15, 2011 at 07:16:10PM -0400, Devin Nakamura wrote:
>> @@ -263,6 +263,8 @@ static int qed_read_string(BlockDriverState *file, uint64_t offset, size_t n,
>>   */
>>  static uint64_t qed_alloc_clusters(BDRVQEDState *s, unsigned int n)
>>  {
>> +    s->file_size =  qed_start_of_cluster(s, s->file_size +
>> +                                         s->header.cluster_size - 1);
>
> Why do you need this?  QED cluster aligns the file size on open.
> file_size should always be cluster-aligned.
>
> Stefan
>
I was running into problems when I was doing in-place conversion from
qcow2. But I suppose I could cluster align the file offset when I open
the conversion target. I just seemed this way was safer.
Kevin Wolf - Aug. 23, 2011, 12:38 p.m.
Am 16.08.2011 01:16, schrieb Devin Nakamura:
> Signed-off-by: Devin Nakamura <devin122@gmail.com>
> ---
>  block/qed.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/block/qed.c b/block/qed.c
> index 333f067..9a1e49c 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -263,6 +263,8 @@ static int qed_read_string(BlockDriverState *file, uint64_t offset, size_t n,
>   */
>  static uint64_t qed_alloc_clusters(BDRVQEDState *s, unsigned int n)
>  {
> +    s->file_size =  qed_start_of_cluster(s, s->file_size +
> +                                         s->header.cluster_size - 1);
>      uint64_t offset = s->file_size;
>      s->file_size += n * s->header.cluster_size;
>      return offset;

Stefan, Devin, have you come to a conclusion about this patch?

Kevin
Stefan Hajnoczi - Aug. 25, 2011, 9:33 a.m.
On Tue, Aug 23, 2011 at 02:38:00PM +0200, Kevin Wolf wrote:
> Am 16.08.2011 01:16, schrieb Devin Nakamura:
> > Signed-off-by: Devin Nakamura <devin122@gmail.com>
> > ---
> >  block/qed.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/block/qed.c b/block/qed.c
> > index 333f067..9a1e49c 100644
> > --- a/block/qed.c
> > +++ b/block/qed.c
> > @@ -263,6 +263,8 @@ static int qed_read_string(BlockDriverState *file, uint64_t offset, size_t n,
> >   */
> >  static uint64_t qed_alloc_clusters(BDRVQEDState *s, unsigned int n)
> >  {
> > +    s->file_size =  qed_start_of_cluster(s, s->file_size +
> > +                                         s->header.cluster_size - 1);
> >      uint64_t offset = s->file_size;
> >      s->file_size += n * s->header.cluster_size;
> >      return offset;
> 
> Stefan, Devin, have you come to a conclusion about this patch?

Yes, I suggested keeping the current contraint that file_size is
cluster-aligned.

Devin, does that work for you?

Stefan
Devin Nakamura - Aug. 25, 2011, 10:14 p.m.
Works fine for me

On Thu, Aug 25, 2011 at 5:33 AM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> On Tue, Aug 23, 2011 at 02:38:00PM +0200, Kevin Wolf wrote:
>> Am 16.08.2011 01:16, schrieb Devin Nakamura:
>> > Signed-off-by: Devin Nakamura <devin122@gmail.com>
>> > ---
>> >  block/qed.c |    2 ++
>> >  1 files changed, 2 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/block/qed.c b/block/qed.c
>> > index 333f067..9a1e49c 100644
>> > --- a/block/qed.c
>> > +++ b/block/qed.c
>> > @@ -263,6 +263,8 @@ static int qed_read_string(BlockDriverState *file, uint64_t offset, size_t n,
>> >   */
>> >  static uint64_t qed_alloc_clusters(BDRVQEDState *s, unsigned int n)
>> >  {
>> > +    s->file_size =  qed_start_of_cluster(s, s->file_size +
>> > +                                         s->header.cluster_size - 1);
>> >      uint64_t offset = s->file_size;
>> >      s->file_size += n * s->header.cluster_size;
>> >      return offset;
>>
>> Stefan, Devin, have you come to a conclusion about this patch?
>
> Yes, I suggested keeping the current contraint that file_size is
> cluster-aligned.
>
> Devin, does that work for you?
>
> Stefan
>

Patch

diff --git a/block/qed.c b/block/qed.c
index 333f067..9a1e49c 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -263,6 +263,8 @@  static int qed_read_string(BlockDriverState *file, uint64_t offset, size_t n,
  */
 static uint64_t qed_alloc_clusters(BDRVQEDState *s, unsigned int n)
 {
+    s->file_size =  qed_start_of_cluster(s, s->file_size +
+                                         s->header.cluster_size - 1);
     uint64_t offset = s->file_size;
     s->file_size += n * s->header.cluster_size;
     return offset;