Re: [Qemu-devel] block: Update flags in bdrv_set_read_only()

Re: [Qemu-devel] block: Update flags in bdrv_set_read_only()

[Michael Tokarev]

     commit eeae6a596b0efc092f5101c67683053e245e6250
     Author: Kevin Wolf <kwolf@redhat.com>
     Date:   Tue Oct 9 16:57:12 2018 +0200

         block: Update flags in bdrv_set_read_only()

         To fully change the read-only state of a node, we must not only change
         bs->read_only, but also update bs->open_flags.

sort of broke vfat support:

  $ qemu-system-x86_64 -hda fat:foo/
  WARNING: Image format was not specified for 'json:{"fat-type": 0, "dir": "foo/", "driver": "vvfat", "floppy": false, "rw": false}' and probing guessed raw.
           Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
           Specify the 'raw' format explicitly to remove the restrictions.
  qemu-system-x86_64: Initialization of device ide-hd failed: Block node is read-only
  $ _

The warning is annoying but harmless, but the read-only error is fatal.

"Sort-of" is because there's a somewhat strange workaround:

   -hda fat:rw:foo/

but it is a bit more dangerous as well.

It looks like vfat should be handled differently somewhere, to
eliminate both the warning and the error?

Thanks,

/mjt

Re: [Qemu-devel] block: Update flags in bdrv_set_read_only()

[Kevin Wolf]

Am 12.01.2019 um 18:08 hat Michael Tokarev geschrieben:
>     commit eeae6a596b0efc092f5101c67683053e245e6250
>     Author: Kevin Wolf <kwolf@redhat.com>
>     Date:   Tue Oct 9 16:57:12 2018 +0200
> 
>         block: Update flags in bdrv_set_read_only()
> 
>         To fully change the read-only state of a node, we must not only change
>         bs->read_only, but also update bs->open_flags.
> 
> sort of broke vfat support:
> 
>  $ qemu-system-x86_64 -hda fat:foo/
>  WARNING: Image format was not specified for 'json:{"fat-type": 0, "dir": "foo/", "driver": "vvfat", "floppy": false, "rw": false}' and probing guessed raw.
>           Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
>           Specify the 'raw' format explicitly to remove the restrictions.
>  qemu-system-x86_64: Initialization of device ide-hd failed: Block node is read-only
>  $ _
> 
> The warning is annoying but harmless, but the read-only error is fatal.
> 
> "Sort-of" is because there's a somewhat strange workaround:
> 
>   -hda fat:rw:foo/
> 
> but it is a bit more dangerous as well.
> 
> It looks like vfat should be handled differently somewhere, to
> eliminate both the warning and the error?

Hm... This is not nice, but obviously that patch is still correct.

Essentially what you're saying is either:

1. We want to be able to attach read-only backends to read-write guest
   devices sometimes. If you actually do a write request then, you'll
   get an I/O error,

   or

2. vvfat shouldn't expose a read-only backend, but a read-write one that
   always fails when you write.

I think 2. is easier to implement, but it's special casing vvfat. Does
this make sense or is it a problem that needs to be solved more
generically? If it's okay for a read-only FAT backend to be attached to
an IDE disk that really needs a read-write backend, why wouldn't it be
okay to attach e.g. a read-only HTTP backend? Or even a read-only image
file on the local filesystem?

On the other hand, usually users wouldn't want to silently get a guest
started up that produces I/O errors on the first write request when they
just configured things wrong or have the wrong file permissions.

We can't do both at the same time, though. So what is the behaviour that
we actually want regarding read-only backends and read-write guest
devices?

Kevin

Re: [Qemu-devel] block: Update flags in bdrv_set_read_only()

[Kevin Wolf]

[ Cc: qemu-block - noticed only now that it was missing ]

Am 14.01.2019 um 12:01 hat Kevin Wolf geschrieben:
> Am 12.01.2019 um 18:08 hat Michael Tokarev geschrieben:
> >     commit eeae6a596b0efc092f5101c67683053e245e6250
> >     Author: Kevin Wolf <kwolf@redhat.com>
> >     Date:   Tue Oct 9 16:57:12 2018 +0200
> > 
> >         block: Update flags in bdrv_set_read_only()
> > 
> >         To fully change the read-only state of a node, we must not only change
> >         bs->read_only, but also update bs->open_flags.
> > 
> > sort of broke vfat support:
> > 
> >  $ qemu-system-x86_64 -hda fat:foo/
> >  WARNING: Image format was not specified for 'json:{"fat-type": 0, "dir": "foo/", "driver": "vvfat", "floppy": false, "rw": false}' and probing guessed raw.
> >           Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
> >           Specify the 'raw' format explicitly to remove the restrictions.
> >  qemu-system-x86_64: Initialization of device ide-hd failed: Block node is read-only
> >  $ _
> > 
> > The warning is annoying but harmless, but the read-only error is fatal.
> > 
> > "Sort-of" is because there's a somewhat strange workaround:
> > 
> >   -hda fat:rw:foo/
> > 
> > but it is a bit more dangerous as well.
> > 
> > It looks like vfat should be handled differently somewhere, to
> > eliminate both the warning and the error?
> 
> Hm... This is not nice, but obviously that patch is still correct.
> 
> Essentially what you're saying is either:
> 
> 1. We want to be able to attach read-only backends to read-write guest
>    devices sometimes. If you actually do a write request then, you'll
>    get an I/O error,
> 
>    or
> 
> 2. vvfat shouldn't expose a read-only backend, but a read-write one that
>    always fails when you write.
> 
> I think 2. is easier to implement, but it's special casing vvfat. Does
> this make sense or is it a problem that needs to be solved more
> generically? If it's okay for a read-only FAT backend to be attached to
> an IDE disk that really needs a read-write backend, why wouldn't it be
> okay to attach e.g. a read-only HTTP backend? Or even a read-only image
> file on the local filesystem?
> 
> On the other hand, usually users wouldn't want to silently get a guest
> started up that produces I/O errors on the first write request when they
> just configured things wrong or have the wrong file permissions.
> 
> We can't do both at the same time, though. So what is the behaviour that
> we actually want regarding read-only backends and read-write guest
> devices?
> 
> Kevin