[Qemu-devel] [PATCH 1/2] qemu-nbd: Shrink image size by specified offset

[Qemu-devel] [PATCH 1/2] qemu-nbd: Shrink image size by specified offset

[Tomáš Golembiovský]

When --offset is set the apparent device size has to be adjusted
accordingly. Otherwise client may request read/write beyond the file end
which would fail.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 qemu-nbd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 99297a5..629bce1 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -901,6 +901,13 @@ int main(int argc, char **argv)
         exit(EXIT_FAILURE);
     }
 
+    if (dev_offset >= fd_size) {
+        error_report("Offset (%lu) has to be smaller than the image size (%lu)",
+                     dev_offset, fd_size);
+        exit(EXIT_FAILURE);
+    }
+    fd_size -= dev_offset;
+
     if (partition != -1) {
         ret = find_partition(blk, partition, &dev_offset, &fd_size);
         if (ret < 0) {
-- 
2.9.3

Re: [Qemu-devel] [PATCH 1/2] qemu-nbd: Shrink image size by specified offset

[Paolo Bonzini]

On 20/09/2016 11:37, Tomáš Golembiovský wrote:
> When --offset is set the apparent device size has to be adjusted
> accordingly. Otherwise client may request read/write beyond the file end
> which would fail.
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  qemu-nbd.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 99297a5..629bce1 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -901,6 +901,13 @@ int main(int argc, char **argv)
>          exit(EXIT_FAILURE);
>      }
>  
> +    if (dev_offset >= fd_size) {
> +        error_report("Offset (%lu) has to be smaller than the image size (%lu)",
> +                     dev_offset, fd_size);
> +        exit(EXIT_FAILURE);
> +    }
> +    fd_size -= dev_offset;
> +
>      if (partition != -1) {
>          ret = find_partition(blk, partition, &dev_offset, &fd_size);
>          if (ret < 0) {
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH 1/2] qemu-nbd: Shrink image size by specified offset

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]

On 09/20/2016 04:37 AM, Tomáš Golembiovský wrote:

[meta-comment]: Your series came through without any threading (you sent
three threads, instead of patch 1 and 2 being marked In-Reply-To the 0/2
cover letter).

> When --offset is set the apparent device size has to be adjusted
> accordingly. Otherwise client may request read/write beyond the file end
> which would fail.
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  qemu-nbd.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 99297a5..629bce1 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -901,6 +901,13 @@ int main(int argc, char **argv)
>          exit(EXIT_FAILURE);
>      }

Additional context:

    off_t dev_offset = 0;

    off_t fd_size;

>  
> +    if (dev_offset >= fd_size) {
> +        error_report("Offset (%lu) has to be smaller than the image size (%lu)",
> +                     dev_offset, fd_size);

Whoops, this fails to compile on 32-bit platforms.  %lu is not
necessarily synonymous with off_t values.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 1/2] qemu-nbd: Shrink image size by specified offset

[Tomáš Golembiovský]

Whops, somehow I completely forgot about this.

On Tue, 20 Sep 2016 09:09:59 -0500
Eric Blake <eblake@redhat.com> wrote:

> On 09/20/2016 04:37 AM, Tomáš Golembiovský wrote:
> 
> [meta-comment]: Your series came through without any threading (you sent
> three threads, instead of patch 1 and 2 being marked In-Reply-To the 0/2
> cover letter).

Thanks for the comment. Unfortunately it was my email client
interfering. It should be better next time.


> > When --offset is set the apparent device size has to be adjusted
> > accordingly. Otherwise client may request read/write beyond the file end
> > which would fail.
> > 
> > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> > ---
> >  qemu-nbd.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/qemu-nbd.c b/qemu-nbd.c
> > index 99297a5..629bce1 100644
> > --- a/qemu-nbd.c
> > +++ b/qemu-nbd.c
> > @@ -901,6 +901,13 @@ int main(int argc, char **argv)
> >          exit(EXIT_FAILURE);
> >      }  
> 
> Additional context:
> 
>     off_t dev_offset = 0;
> 
>     off_t fd_size;
> 
> >  
> > +    if (dev_offset >= fd_size) {
> > +        error_report("Offset (%lu) has to be smaller than the image size (%lu)",
> > +                     dev_offset, fd_size);  
> 
> Whoops, this fails to compile on 32-bit platforms.  %lu is not
> necessarily synonymous with off_t values.

After some digging I figured off_t is in fact signed type. That makes
the formatting wrong everywhere. Unfortunately I didn't find any good
definition of the type. Any suggestion what format flag should I use? Or
should I just use a temporary variable of known size for that?


Thanks,

    Tomas


-- 
Tomáš Golembiovský <tgolembi@redhat.com>

Re: [Qemu-devel] [PATCH 1/2] qemu-nbd: Shrink image size by specified offset

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]

On 10/03/2016 08:50 AM, Tomáš Golembiovský wrote:
>> Additional context:
>>
>>     off_t dev_offset = 0;
>>
>>     off_t fd_size;
>>
>>>  
>>> +    if (dev_offset >= fd_size) {
>>> +        error_report("Offset (%lu) has to be smaller than the image size (%lu)",
>>> +                     dev_offset, fd_size);  
>>
>> Whoops, this fails to compile on 32-bit platforms.  %lu is not
>> necessarily synonymous with off_t values.
> 
> After some digging I figured off_t is in fact signed type. That makes
> the formatting wrong everywhere. Unfortunately I didn't find any good
> definition of the type. Any suggestion what format flag should I use? Or
> should I just use a temporary variable of known size for that?

Easiest is probably casting to a type with an easier format flag, as in
either of:

error_report("offset %lld ...", (long long) dev_offset)
error_report("offset %jd ...", (intmax_t) dev_offset)

off_t is particularly problematic because there is no magic % sequence
reserved for it, nothing in <inttypes.h> for it, and there are 32-bit
compilation environments where it is still 32 bits (although qemu
prefers to explicitly request large-file compilation so that off_t is
always 64 bits)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]