[PATCH] libxl: No emulated disk driver for xvdX disk

[PATCH] libxl: No emulated disk driver for xvdX disk

[Anthony PERARD]

When a guest configuration list xvdX for its disks, there is no need to
provide an emulated driver for the same target.

Such configuration can work with the OVMF firmware, as it supports PV
disk.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_dm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 71a1a3e..5371f72 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1190,6 +1190,12 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                     drive = libxl__sprintf
                         (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback",
                          pdev_path, disk, format);
+                else if (strncmp(disks[i].vdev, "xvd", 3) == 0)
+                    /*
+                     * Do not add any emulated disk when PV disk are
+                     * explicitly asked for.
+                     */
+                    continue;
                 else if (disk < 6 && b_info->u.hvm.hdtype == LIBXL_HDTYPE_AHCI) {
                     flexarray_vappend(dm_args, "-drive",
                         GCSPRINTF("file=%s,if=none,id=ahcidisk-%d,format=%s,cache=writeback",
-- 
Anthony PERARD

Re: [PATCH] libxl: No emulated disk driver for xvdX disk

[Ian Jackson]

Anthony PERARD writes ("[PATCH] libxl: No emulated disk driver for xvdX disk"):
> When a guest configuration list xvdX for its disks, there is no need to
> provide an emulated driver for the same target.
> 
> Such configuration can work with the OVMF firmware, as it supports PV
> disk.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

I have checked this with the docs, and this change makes the libxl
behave as documented and as I intended when I wrote the doc.

Ian.

Re: [PATCH] libxl: No emulated disk driver for xvdX disk

[Ian Campbell]

On Fri, 2015-10-16 at 15:01 +0100, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH] libxl: No emulated disk driver for xvdX
> disk"):
> > When a guest configuration list xvdX for its disks, there is no need to
> > provide an emulated driver for the same target.
> > 
> > Such configuration can work with the OVMF firmware, as it supports PV
> > disk.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Applied.

Re: [PATCH] libxl: No emulated disk driver for xvdX disk

[Boris Ostrovsky]

On 10/16/2015 10:01 AM, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH] libxl: No emulated disk driver for xvdX disk"):
>> When a guest configuration list xvdX for its disks, there is no need to
>> provide an emulated driver for the same target.
>>
>> Such configuration can work with the OVMF firmware, as it supports PV
>> disk.
> Acked-by: Ian Jackson<ian.jackson@eu.citrix.com>
>
> I have checked this with the docs, and this change makes the libxl
> behave as documented and as I intended when I wrote the doc.
>


This commit prevents a guest from discovering disks that specify xvdX 
("disk = [ '/root/virt/fedora.img,raw,xvda,rw' ]", for example).

TBH, I am not sure I understand how this is supposed to work --- how is 
the guest expected to discover the disk?

-boris

Re: [PATCH] libxl: No emulated disk driver for xvdX disk

[Boris Ostrovsky]

On 10/16/2015 10:01 AM, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH] libxl: No emulated disk driver for xvdX disk"):
>> When a guest configuration list xvdX for its disks, there is no need to
>> provide an emulated driver for the same target.
>>
>> Such configuration can work with the OVMF firmware, as it supports PV
>> disk.
> Acked-by: Ian Jackson<ian.jackson@eu.citrix.com>
>
> I have checked this with the docs, and this change makes the libxl
> behave as documented and as I intended when I wrote the doc.
>


This commit prevents a guest from seeing disks that specify xvdX ("disk 
= [ '/root/virt/fedora.img,raw,xvda,rw' ]", for example).

TBH, I am not sure I understand how this is supposed to work --- how is 
the guest expected to discover the disk?

-boris

Re: [PATCH] libxl: No emulated disk driver for xvdX disk

[Stefano Stabellini]

On Mon, 26 Oct 2015, Boris Ostrovsky wrote:
> On 10/16/2015 10:01 AM, Ian Jackson wrote:
> > Anthony PERARD writes ("[PATCH] libxl: No emulated disk driver for xvdX
> > disk"):
> > > When a guest configuration list xvdX for its disks, there is no need to
> > > provide an emulated driver for the same target.
> > > 
> > > Such configuration can work with the OVMF firmware, as it supports PV
> > > disk.
> > Acked-by: Ian Jackson<ian.jackson@eu.citrix.com>
> > 
> > I have checked this with the docs, and this change makes the libxl
> > behave as documented and as I intended when I wrote the doc.
> > 
> 
> 
> This commit prevents a guest from seeing disks that specify xvdX ("disk = [
> '/root/virt/fedora.img,raw,xvda,rw' ]", for example).
> 
> TBH, I am not sure I understand how this is supposed to work --- how is the
> guest expected to discover the disk?

The guest can access xvdX disks via the PV block protocol, or via
UEFI firmare using EFI BootServices.

Re: [PATCH] libxl: No emulated disk driver for xvdX disk

[George Dunlap]

On Tue, Oct 27, 2015 at 11:11 AM, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Mon, 26 Oct 2015, Boris Ostrovsky wrote:
>> On 10/16/2015 10:01 AM, Ian Jackson wrote:
>> > Anthony PERARD writes ("[PATCH] libxl: No emulated disk driver for xvdX
>> > disk"):
>> > > When a guest configuration list xvdX for its disks, there is no need to
>> > > provide an emulated driver for the same target.
>> > >
>> > > Such configuration can work with the OVMF firmware, as it supports PV
>> > > disk.
>> > Acked-by: Ian Jackson<ian.jackson@eu.citrix.com>
>> >
>> > I have checked this with the docs, and this change makes the libxl
>> > behave as documented and as I intended when I wrote the doc.
>> >
>>
>>
>> This commit prevents a guest from seeing disks that specify xvdX ("disk = [
>> '/root/virt/fedora.img,raw,xvda,rw' ]", for example).
>>
>> TBH, I am not sure I understand how this is supposed to work --- how is the
>> guest expected to discover the disk?
>
> The guest can access xvdX disks via the PV block protocol, or via
> UEFI firmare using EFI BootServices.

But it introduces a sort of regression in some setups -- namely,
before you could use exactly the same disk and config file for either
a PV or HVM guest, simply adding builder=hvm (either to the file or
the command line) to switch between the two.

I'm not sure how many people depend on disk=[...xvda] working on
non-OVMF HVM guests, but my current testing setups certainly do.

 -George

Re: [PATCH] libxl: No emulated disk driver for xvdX disk

[Ian Jackson]

George Dunlap writes ("Re: [Xen-devel] [PATCH] libxl: No emulated disk driver for xvdX disk"):
> On Tue, Oct 27, 2015 at 11:11 AM, Stefano Stabellini
> <stefano.stabellini@eu.citrix.com> wrote:
> > The guest can access xvdX disks via the PV block protocol, or via
> > UEFI firmare using EFI BootServices.
> 
> But it introduces a sort of regression in some setups -- namely,
> before you could use exactly the same disk and config file for either
> a PV or HVM guest, simply adding builder=hvm (either to the file or
> the command line) to switch between the two.

The purpose of the `vdev' in the xl disk specification is to say how
the disk should be presented to the guest.

Did you look at docs/misc/vbd-interface.txt ?

If you want something that is accessible via the BIOS etc. in HVM
guests, why not use vdev=hda rather than vdev=xvda ?

Ian.

Re: [PATCH] libxl: No emulated disk driver for xvdX disk

[Boris Ostrovsky]

On 10/27/2015 09:19 AM, Ian Jackson wrote:
> George Dunlap writes ("Re: [Xen-devel] [PATCH] libxl: No emulated disk driver for xvdX disk"):
>> On Tue, Oct 27, 2015 at 11:11 AM, Stefano Stabellini
>> <stefano.stabellini@eu.citrix.com> wrote:
>>> The guest can access xvdX disks via the PV block protocol, or via
>>> UEFI firmare using EFI BootServices.
>> But it introduces a sort of regression in some setups -- namely,
>> before you could use exactly the same disk and config file for either
>> a PV or HVM guest, simply adding builder=hvm (either to the file or
>> the command line) to switch between the two.
> The purpose of the `vdev' in the xl disk specification is to say how
> the disk should be presented to the guest.
>
> Did you look at docs/misc/vbd-interface.txt ?
>
> If you want something that is accessible via the BIOS etc. in HVM
> guests, why not use vdev=hda rather than vdev=xvda ?

Yes, but since we've discovered that people do use xvda (and have been 
doing this for long time --- our test framework has been running for 
many years configured like that) --- do we want this to suddenly break?

-boris

Re: [PATCH] libxl: No emulated disk driver for xvdX disk

[Ian Jackson]

Boris Ostrovsky writes ("Re: [Xen-devel] [PATCH] libxl: No emulated disk driver for xvdX disk"):
> On 10/27/2015 09:19 AM, Ian Jackson wrote:
> > If you want something that is accessible via the BIOS etc. in HVM
> > guests, why not use vdev=hda rather than vdev=xvda ?
> 
> Yes, but since we've discovered that people do use xvda (and have been 
> doing this for long time --- our test framework has been running for 
> many years configured like that) --- do we want this to suddenly break?

So you want us to change the spec to match the implementation ?
(I haven't looked what happens with trad qemu.)

If we change the spec, how would a user request an xvda with no
emulated device ?

Ian.

Re: [PATCH] libxl: No emulated disk driver for xvdX disk

[George Dunlap]

On 27/10/15 13:19, Ian Jackson wrote:
> George Dunlap writes ("Re: [Xen-devel] [PATCH] libxl: No emulated disk driver for xvdX disk"):
>> On Tue, Oct 27, 2015 at 11:11 AM, Stefano Stabellini
>> <stefano.stabellini@eu.citrix.com> wrote:
>>> The guest can access xvdX disks via the PV block protocol, or via
>>> UEFI firmare using EFI BootServices.
>>
>> But it introduces a sort of regression in some setups -- namely,
>> before you could use exactly the same disk and config file for either
>> a PV or HVM guest, simply adding builder=hvm (either to the file or
>> the command line) to switch between the two.
> 
> The purpose of the `vdev' in the xl disk specification is to say how
> the disk should be presented to the guest.
> 
> Did you look at docs/misc/vbd-interface.txt ?
> 
> If you want something that is accessible via the BIOS etc. in HVM
> guests, why not use vdev=hda rather than vdev=xvda ?

If I then take the came config file and remove 'builder="hvm"', will the
resulting thing boot as a PV guest?

I understand the idea of saying, "Well, 'vdev=xvda' means a PV disk; you
haven't asked for an emulated disk, so you shouldn't expect to get one."
 It's all very nice and neat and logical.  But:

1. It's been this way for years, and at least many developers' testing
systems have come to rely on this behavior.  I'd be surprised if this
didn't break a lot of users' configurations on upgrade

2. Being able to boot the exact same config file either PV or HVM is a
feature that makes sense to have

If both SeaBIOS and OVMF had PV drivers I don't think it would really be
an issue; but they don't.

The real issue I guess is that there is "automatic" behavior here that
we don't have a flag to control: PV disks get emulated counterparts
automatically, and emulated disks get PV counterparts automatically.
Would it make sense to have a per-device flag to specify this?
mirror=(true|false|auto), where "auto" is the default?

If we can detect whether we're booting with SeaBIOS or OVMF, we can even
have "auto" default to "false' for OVMF, and "true" for SeaBIOS (until
such time as we have PV drivers in SeaBIOS).

 -George

Re: [PATCH] libxl: No emulated disk driver for xvdX disk

[Boris Ostrovsky]

On 10/27/2015 10:26 AM, Ian Jackson wrote:
> Boris Ostrovsky writes ("Re: [Xen-devel] [PATCH] libxl: No emulated disk driver for xvdX disk"):
>> On 10/27/2015 09:19 AM, Ian Jackson wrote:
>>> If you want something that is accessible via the BIOS etc. in HVM
>>> guests, why not use vdev=hda rather than vdev=xvda ?
>> Yes, but since we've discovered that people do use xvda (and have been
>> doing this for long time --- our test framework has been running for
>> many years configured like that) --- do we want this to suddenly break?
> So you want us to change the spec to match the implementation ?
> (I haven't looked what happens with trad qemu.)
>
> If we change the spec, how would a user request an xvda with no
> emulated device ?

Can we allow specifying xvdX when we know that seabios is going to be used?

-boris

Re: [PATCH] libxl: No emulated disk driver for xvdX disk

[Ian Jackson]

George Dunlap writes ("Re: [Xen-devel] [PATCH] libxl: No emulated disk driver for xvdX disk"):
> If I then take the came config file and remove 'builder="hvm"', will the
> resulting thing boot as a PV guest?

I haven't tried this but I don't see why not.

> The real issue I guess is that there is "automatic" behavior here that
> we don't have a flag to control: PV disks get emulated counterparts
> automatically, and emulated disks get PV counterparts automatically.
> Would it make sense to have a per-device flag to specify this?
> mirror=(true|false|auto), where "auto" is the default?
> 
> If we can detect whether we're booting with SeaBIOS or OVMF, we can even
> have "auto" default to "false' for OVMF, and "true" for SeaBIOS (until
> such time as we have PV drivers in SeaBIOS).

I don't think this is helpful.

Ian.

Re: [PATCH] libxl: No emulated disk driver for xvdX disk

[George Dunlap]

On 27/10/15 14:43, Ian Jackson wrote:
> George Dunlap writes ("Re: [Xen-devel] [PATCH] libxl: No emulated disk driver for xvdX disk"):
>> If I then take the came config file and remove 'builder="hvm"', will the
>> resulting thing boot as a PV guest?
> 
> I haven't tried this but I don't see why not.

FWIW it does work.

 -George