Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"

[Stefan Hajnoczi]

On Tue, Feb 21, 2017 at 05:21:46PM +0000, Ketan Nilangekar wrote:
> 
> 
> On 2/21/17, 5:59 AM, "Stefan Hajnoczi" <address@hidden> wrote:
> 
>     On Mon, Feb 20, 2017 at 03:34:57AM -0800, ashish mittal wrote:
>     > On Mon, Feb 20, 2017 at 3:02 AM, Stefan Hajnoczi <address@hidden> wrote:
>     > > On Sat, Feb 18, 2017 at 12:30:31AM +0000, Ketan Nilangekar wrote:
>     > >> On 2/17/17, 1:42 PM, "Jeff Cody" <address@hidden> wrote:
>     > >>
>     > >>     On Thu, Feb 16, 2017 at 02:24:19PM -0800, ashish mittal wrote:
>     > >>     > Hi,
>     > >>     >
>     > >>     > I am getting the following error with checkpatch.pl
>     > >>     >
>     > >>     > ERROR: externs should be avoided in .c files
>     > >>     > #78: FILE: block/vxhs.c:28:
>     > >>     > +QemuUUID qemu_uuid __attribute__ ((weak));
>     > >>     >
>     > >>     > Is there any way to get around this, or does it mean that I 
> would have
>     > >>     > to add a vxhs.h just for this one entry?
>     > >>     >
>     > >>
>     > >>     I remain skeptical on the use of the qemu_uuid as a way to 
> select the TLS
>     > >>     cert.
>     > >>
>     > >> [ketan]
>     > >> Is there another identity that can be used for uniquely identifying 
> instances?
>     > >> The requirement was to enforce vdisk access to owner instances.
>     > >
>     > > The qemu_uuid weak attribute looks suspect.  What is going to provide 
> a
>     > > strong qemu_uuid symbol?
>     > >
>     > > Why aren't configuration parameters like the UUID coming from the QEMU
>     > > command-line?
>     > >
>     > > Stefan
>     > 
>     > UUID will in fact come from the QEMU command line. VxHS is not doing
>     > anything special here. It will just use the value already available to
>     > qemu-kvm process.
>     > 
>     > QemuUUID qemu_uuid;
>     > bool qemu_uuid_set;
>     > 
>     > Both the above are defined in vl.c. vl.c will provide the strong
>     > symbol when available. There are certain binaries that do not get
>     > linked with vl.c (e.g. qemu-img). The weak symbol will come into
>     > affect for such binaries, and in this case, the default VXHS UUID will
>     > get picked up. I had, in a previous email, explained how we plan to
>     > use the default UUID. In the regular case, the VxHS controller will
>     > not allow access to the default UUID (non qemu-kvm) binaries, but it
>     > may choose to grant temporary access to specific vdisks for these
>     > binaries depending on the workflow.
>     
>     That idea sounds like a security problem.  During this time window
>     anyone could use the default UUID to access the data?
>     
>     Just make the UUID (or TLS client certificate file) a command-line
>     parameter that qemu-system, qemu-img, and other tools accept (e.g.
>     qemu-img via the --image-opts/--object syntax).
>     
> [Ketan]
> Sounds fair. Would it be ok to take this up after the driver is merged for 
> the upcoming QEMU release?

No, because the current command-line interface isn't production-ready.
QEMU's command-line interface must stay backwards compatible forever so
merging the incomplete code now will create problems.

Start by tackling the comments that Daniel Berrange made 9 days ago
about hardcoded paths in libvxhs:
https://github.com/VeritasHyperScale/libqnio/pull/12#discussion_r100812640

As a consequence of resolving that issue, the QEMU block driver will
need new command-line parameters so it can pass in the configuration.
Then qemu_uuid will not be needed anymore because the command-line
parameters should already specify everything that libvxhs needs.

Stefan

signature.asc
Description: PGP signature