Re: [Qemu-devel] [PATCH] raw-posix.c: remove raw device access for cdrom

[Kevin Wolf <kwolf@redhat.com>]

Am 08.07.2015 um 14:56 hat Programmingkid geschrieben:
> 
> On Jul 8, 2015, at 7:01 AM, Kevin Wolf wrote:
> 
> > Am 08.07.2015 um 12:47 hat Laurent Vivier geschrieben:
> >> 
> >> 
> >> On 08/07/2015 12:31, Kevin Wolf wrote:
> >>> Am 02.07.2015 um 16:18 hat Laurent Vivier geschrieben:
> >>>> 
> >>>> 
> >>>> On 02/07/2015 16:03, Paolo Bonzini wrote:
> >>>>> 
> >>>>> 
> >>>>> On 02/07/2015 15:58, Laurent Vivier wrote:
> >>>>>> Since any /dev entry can be treated as a raw disk image, it is worth
> >>>>>> noting which devices can be accessed when and how. /dev/rdisk nodes are
> >>>>>> character-special devices, but are "raw" in the BSD sense and force
> >>>>>> block-aligned I/O. They are closer to the physical disk than the buffer
> >>>>>> cache. /dev/disk nodes, on the other hand, are buffered block-special
> >>>>>> devices and are used primarily by the kernel's filesystem code.
> >>>>> 
> >>>>> So the right thing to do would not be just to set need_alignment, but to
> >>>>> probe it like we do on Linux for BDRV_O_NO_CACHE.
> >>>>> 
> >>>>> I'm okay with doing the simple thing, but it needs a comment for non-BSDers.
> >>>> 
> >>>> So, what we have to do, in our case, for MacOS X cdrom, is something like:
> >>>> 
> >>>> ... GetBSDPath ...
> >>>> ...
> >>>>    if (flags & BDRV_O_NOCACHE) {
> >>>>        strcat(bsdPath, "r");
> >>>>    }
> >>>> ...
> >>> 
> >>> I would avoid such magic. What we could do is rejecting /dev/rdisk nodes
> >>> without BDRV_O_NOCACHE.
> >> 
> >> It's not how it works...
> >> 
> >> Look in hdev_open().
> >> 
> >> If user provides /dev/cdrom on the command line, in the case of MacOS X,
> >> QEMU searches for a cdrom drive in the system and set filename to
> >> /dev/rdiskX according to the result.
> > 
> > Oh, we're already playing such games... I guess you're right then.
> > 
> > It even seems to be not only for '/dev/cdrom', but for everything
> > starting with this string. Does anyone know what's the reason for that?
> > 
> > Also, I guess before doing strcat() on bsdPath, we should check the
> > buffer length...
> 
> By buffer, do you mean the bsdPath variable?

Yes. In theory, bsdPath could be completely filled with the path
returned by GetBSDPath() because we pass sizeof(bsdPath) as maxPathSize.
Appending "s0" would then overflow the buffer.

I'll admit that this is rather unlikely to happen, but being careful
when dealing with strings can never hurt.

Kevin