$ git checkout RELEASE-4.2.2 $ git clean -ffdx [do a build, install, reboot] $ xl create a0 $ xl cd-eject a0 hdc [However, the guest OS still sees the CD inserted] $ xl block-list a0 Segmentation fault $ xl cd-insert a0 hdc file:/images/xs-tools-6.0.0.iso Segmentation fault The problem seems to be that xl cd-eject is writing corrupt values to xenstore. Note that vbd/1/5632 is still there as when it booted, but qdisk/1/5632 has some random stuff in it $ xenstore-ls -f /local/domain/0/backend /local/domain/0/backend/qdisk = "" /local/domain/0/backend/qdisk/1 = "" /local/domain/0/backend/qdisk/1/768 = "" /local/domain/0/backend/qdisk/1/768/frontend = "/local/domain/1/device/vbd/768" /local/domain/0/backend/qdisk/1/768/params = "qcow:/images/vm/a0" /local/domain/0/backend/qdisk/1/768/frontend-id = "1" /local/domain/0/backend/qdisk/1/768/online = "1" /local/domain/0/backend/qdisk/1/768/removable = "0" /local/domain/0/backend/qdisk/1/768/bootable = "1" /local/domain/0/backend/qdisk/1/768/state = "2" /local/domain/0/backend/qdisk/1/768/dev = "hda" /local/domain/0/backend/qdisk/1/768/type = "qdisk" /local/domain/0/backend/qdisk/1/768/mode = "w" /local/domain/0/backend/qdisk/1/768/device-type = "disk" /local/domain/0/backend/qdisk/1/768/feature-barrier = "1" /local/domain/0/backend/qdisk/1/768/info = "0" /local/domain/0/backend/qdisk/1/768/sector-size = "512" /local/domain/0/backend/qdisk/1/768/sectors = "33554432" /local/domain/0/backend/qdisk/1/768/hotplug-status = "connected" /local/domain/0/backend/qdisk/1/5632 = "" /local/domain/0/backend/qdisk/1/5632/type = "qdisk" /local/domain/0/backend/qdisk/1/5632/params = "" /local/domain/0/backend/vbd = "" /local/domain/0/backend/vbd/1 = "" /local/domain/0/backend/vbd/1/5632 = "" /local/domain/0/backend/vbd/1/5632/frontend = "/local/domain/1/device/vbd/5632" /local/domain/0/backend/vbd/1/5632/tapdisk-params "aio:/images/autoinstall//w2k3eesp2.iso" /local/domain/0/backend/vbd/1/5632/params = "/dev/xen/blktap-2/tapdev0" /local/domain/0/backend/vbd/1/5632/script = "/etc/xen/scripts/block" /local/domain/0/backend/vbd/1/5632/physical-device = "fd:0" /local/domain/0/backend/vbd/1/5632/frontend-id = "1" /local/domain/0/backend/vbd/1/5632/online = "1" /local/domain/0/backend/vbd/1/5632/removable = "1" /local/domain/0/backend/vbd/1/5632/bootable = "1" /local/domain/0/backend/vbd/1/5632/state = "2" /local/domain/0/backend/vbd/1/5632/dev = "hdc" /local/domain/0/backend/vbd/1/5632/type = "phy" /local/domain/0/backend/vbd/1/5632/mode = "r" /local/domain/0/backend/vbd/1/5632/device-type = "cdrom" The disk string from the config file is as follows: disk = [ ''qcow:/images/vm/a0,hda,w'',''file:/images/autoinstall//w2k3eesp2.iso,hdc:cdrom,r'' ] -George
Ha! I knew I''d seen this before... the following git c/s needs to be
backported:
commit c3556e2a1aee3c9b7dda5d57e85e8867fff1b9da
Author: George Dunlap <george.dunlap@eu.citrix.com>
Commit: George Dunlap <george.dunlap@eu.citrix.com>
libxl: Fix bug in libxl_cdrom_insert, make more robust against bad
xenstore data
libxl_cdrom_insert was failing to initialize the backend type,
resulting in the wrong default backend. The result was not only that
the CD was not inserted properly, but also that some improper xenstore
entries were created, causing further block commands to fail.
This patch fixes the bug by setting the disk backend type based on the
type of the existing device.
It also makes the system more robust by checking to see that it has
got a valid path before proceeding to write a partial xenstore entry.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
On Fri, Apr 26, 2013 at 3:34 PM, George Dunlap
<George.Dunlap@eu.citrix.com> wrote:> $ git checkout RELEASE-4.2.2
> $ git clean -ffdx
>
> [do a build, install, reboot]
>
> $ xl create a0
>
> $ xl cd-eject a0 hdc
>
> [However, the guest OS still sees the CD inserted]
>
> $ xl block-list a0
> Segmentation fault
>
> $ xl cd-insert a0 hdc file:/images/xs-tools-6.0.0.iso
> Segmentation fault
>
> The problem seems to be that xl cd-eject is writing corrupt values to
> xenstore. Note that vbd/1/5632 is still there as when it booted, but
> qdisk/1/5632 has some random stuff in it
>
> $ xenstore-ls -f /local/domain/0/backend
> /local/domain/0/backend/qdisk = ""
> /local/domain/0/backend/qdisk/1 = ""
> /local/domain/0/backend/qdisk/1/768 = ""
> /local/domain/0/backend/qdisk/1/768/frontend =
"/local/domain/1/device/vbd/768"
> /local/domain/0/backend/qdisk/1/768/params = "qcow:/images/vm/a0"
> /local/domain/0/backend/qdisk/1/768/frontend-id = "1"
> /local/domain/0/backend/qdisk/1/768/online = "1"
> /local/domain/0/backend/qdisk/1/768/removable = "0"
> /local/domain/0/backend/qdisk/1/768/bootable = "1"
> /local/domain/0/backend/qdisk/1/768/state = "2"
> /local/domain/0/backend/qdisk/1/768/dev = "hda"
> /local/domain/0/backend/qdisk/1/768/type = "qdisk"
> /local/domain/0/backend/qdisk/1/768/mode = "w"
> /local/domain/0/backend/qdisk/1/768/device-type = "disk"
> /local/domain/0/backend/qdisk/1/768/feature-barrier = "1"
> /local/domain/0/backend/qdisk/1/768/info = "0"
> /local/domain/0/backend/qdisk/1/768/sector-size = "512"
> /local/domain/0/backend/qdisk/1/768/sectors = "33554432"
> /local/domain/0/backend/qdisk/1/768/hotplug-status = "connected"
> /local/domain/0/backend/qdisk/1/5632 = ""
> /local/domain/0/backend/qdisk/1/5632/type = "qdisk"
> /local/domain/0/backend/qdisk/1/5632/params = ""
> /local/domain/0/backend/vbd = ""
> /local/domain/0/backend/vbd/1 = ""
> /local/domain/0/backend/vbd/1/5632 = ""
> /local/domain/0/backend/vbd/1/5632/frontend =
"/local/domain/1/device/vbd/5632"
> /local/domain/0/backend/vbd/1/5632/tapdisk-params >
"aio:/images/autoinstall//w2k3eesp2.iso"
> /local/domain/0/backend/vbd/1/5632/params =
"/dev/xen/blktap-2/tapdev0"
> /local/domain/0/backend/vbd/1/5632/script =
"/etc/xen/scripts/block"
> /local/domain/0/backend/vbd/1/5632/physical-device = "fd:0"
> /local/domain/0/backend/vbd/1/5632/frontend-id = "1"
> /local/domain/0/backend/vbd/1/5632/online = "1"
> /local/domain/0/backend/vbd/1/5632/removable = "1"
> /local/domain/0/backend/vbd/1/5632/bootable = "1"
> /local/domain/0/backend/vbd/1/5632/state = "2"
> /local/domain/0/backend/vbd/1/5632/dev = "hdc"
> /local/domain/0/backend/vbd/1/5632/type = "phy"
> /local/domain/0/backend/vbd/1/5632/mode = "r"
> /local/domain/0/backend/vbd/1/5632/device-type = "cdrom"
>
> The disk string from the config file is as follows:
>
> disk = [
''qcow:/images/vm/a0,hda,w'',''file:/images/autoinstall//w2k3eesp2.iso,hdc:cdrom,r''
> ]
>
> -George
On Fri, 2013-04-26 at 15:34 +0100, George Dunlap wrote:> $ git checkout RELEASE-4.2.2 > $ git clean -ffdx > > [do a build, install, reboot] > > $ xl create a0 > > $ xl cd-eject a0 hdc > > [However, the guest OS still sees the CD inserted] > > $ xl block-list a0 > Segmentation fault > > $ xl cd-insert a0 hdc file:/images/xs-tools-6.0.0.iso > Segmentation faultCan you get a backtrace out of gdb?> The problem seems to be that xl cd-eject is writing corrupt values to > xenstore. Note that vbd/1/5632 is still there as when it booted, but > qdisk/1/5632 has some random stuff in it > > $ xenstore-ls -f /local/domain/0/backend > /local/domain/0/backend/qdisk = "" > /local/domain/0/backend/qdisk/1 = "" > /local/domain/0/backend/qdisk/1/768 = "" > /local/domain/0/backend/qdisk/1/768/frontend = "/local/domain/1/device/vbd/768" > /local/domain/0/backend/qdisk/1/768/params = "qcow:/images/vm/a0" > /local/domain/0/backend/qdisk/1/768/frontend-id = "1" > /local/domain/0/backend/qdisk/1/768/online = "1" > /local/domain/0/backend/qdisk/1/768/removable = "0" > /local/domain/0/backend/qdisk/1/768/bootable = "1" > /local/domain/0/backend/qdisk/1/768/state = "2" > /local/domain/0/backend/qdisk/1/768/dev = "hda" > /local/domain/0/backend/qdisk/1/768/type = "qdisk" > /local/domain/0/backend/qdisk/1/768/mode = "w" > /local/domain/0/backend/qdisk/1/768/device-type = "disk" > /local/domain/0/backend/qdisk/1/768/feature-barrier = "1" > /local/domain/0/backend/qdisk/1/768/info = "0" > /local/domain/0/backend/qdisk/1/768/sector-size = "512" > /local/domain/0/backend/qdisk/1/768/sectors = "33554432" > /local/domain/0/backend/qdisk/1/768/hotplug-status = "connected" > /local/domain/0/backend/qdisk/1/5632 = "" > /local/domain/0/backend/qdisk/1/5632/type = "qdisk" > /local/domain/0/backend/qdisk/1/5632/params = "" > /local/domain/0/backend/vbd = "" > /local/domain/0/backend/vbd/1 = "" > /local/domain/0/backend/vbd/1/5632 = "" > /local/domain/0/backend/vbd/1/5632/frontend = "/local/domain/1/device/vbd/5632" > /local/domain/0/backend/vbd/1/5632/tapdisk-params > "aio:/images/autoinstall//w2k3eesp2.iso" > /local/domain/0/backend/vbd/1/5632/params = "/dev/xen/blktap-2/tapdev0" > /local/domain/0/backend/vbd/1/5632/script = "/etc/xen/scripts/block" > /local/domain/0/backend/vbd/1/5632/physical-device = "fd:0" > /local/domain/0/backend/vbd/1/5632/frontend-id = "1" > /local/domain/0/backend/vbd/1/5632/online = "1" > /local/domain/0/backend/vbd/1/5632/removable = "1" > /local/domain/0/backend/vbd/1/5632/bootable = "1" > /local/domain/0/backend/vbd/1/5632/state = "2" > /local/domain/0/backend/vbd/1/5632/dev = "hdc" > /local/domain/0/backend/vbd/1/5632/type = "phy" > /local/domain/0/backend/vbd/1/5632/mode = "r" > /local/domain/0/backend/vbd/1/5632/device-type = "cdrom" > > The disk string from the config file is as follows: > > disk = [ ''qcow:/images/vm/a0,hda,w'',''file:/images/autoinstall//w2k3eesp2.iso,hdc:cdrom,r'' > ] > > -George
On 26/04/13 15:39, Ian Campbell wrote:> On Fri, 2013-04-26 at 15:34 +0100, George Dunlap wrote: >> $ git checkout RELEASE-4.2.2 >> $ git clean -ffdx >> >> [do a build, install, reboot] >> >> $ xl create a0 >> >> $ xl cd-eject a0 hdc >> >> [However, the guest OS still sees the CD inserted] >> >> $ xl block-list a0 >> Segmentation fault >> >> $ xl cd-insert a0 hdc file:/images/xs-tools-6.0.0.iso >> Segmentation fault > Can you get a backtrace out of gdb?Turns out I already fixed this once! commit c3556e2a1aee3c9b7dda5d57e85e8867fff1b9da Author: George Dunlap <george.dunlap@eu.citrix.com> Commit: George Dunlap <george.dunlap@eu.citrix.com> libxl: Fix bug in libxl_cdrom_insert, make more robust against bad xenstore data libxl_cdrom_insert was failing to initialize the backend type, resulting in the wrong default backend. The result was not only that the CD was not inserted properly, but also that some improper xenstore entries were created, causing further block commands to fail. This patch fixes the bug by setting the disk backend type based on the type of the existing device. It also makes the system more robust by checking to see that it has got a valid path before proceeding to write a partial xenstore entry. Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Committed-by: Ian Campbell <ian.campbell@citrix.com>
George Dunlap writes ("Re: BUG 4.2.2: xl cd-insert corrupts xenstore
state"):> Turns out I already fixed this once!
>
> commit c3556e2a1aee3c9b7dda5d57e85e8867fff1b9da
> Author: George Dunlap <george.dunlap@eu.citrix.com>
> Commit: George Dunlap <george.dunlap@eu.citrix.com>
Here''s the backport, which was nearly trivial.
I''ll push this to 4.2 staging unless someone objevts.
Ian.
commit 1eea2f30f79a1af817fe32c9e01673da1f31dd5f
Author: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Wed May 1 16:56:54 2013 +0100
libxl: Fix bug in libxl_cdrom_insert, make more robust against bad xenstore
data
libxl_cdrom_insert was failing to initialize the backend type,
resulting in the wrong default backend. The result was not only that
the CD was not inserted properly, but also that some improper xenstore
entries were created, causing further block commands to fail.
This patch fixes the bug by setting the disk backend type based on the
type of the existing device.
It also makes the system more robust by checking to see that it has
got a valid path before proceeding to write a partial xenstore entry.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit c3556e2a1aee3c9b7dda5d57e85e8867fff1b9da)
Conflicts:
tools/libxl/libxl.c
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index a6e9601..9f04e36 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2135,6 +2135,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *disk,
libxl__device device;
const char * path;
+ char * tmp;
flexarray_t *insert = NULL;
@@ -2171,8 +2172,11 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *disk,
disks = libxl_device_disk_list(ctx, domid, &num);
for (i = 0; i < num; i++) {
if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
- /* found */
+ {
+ /* Found. Set backend type appropriately. */
+ disk->backend=disks[i].backend;
break;
+ }
}
if (i == num) {
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual device not
found");
@@ -2192,6 +2196,16 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *disk,
if (rc) goto out;
path = libxl__device_backend_path(gc, &device);
+ /* Sanity check: make sure the backend exists before writing here */
+ tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc,
"%s/frontend", path));
+ if (!tmp)
+ {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Internal error: %s does not
exist",
+ libxl__sprintf(gc, "%s/frontend", path));
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
insert = flexarray_make(4, 1);
flexarray_append_pair(insert, "type",
On 01/05/13 16:59, Ian Jackson wrote:> George Dunlap writes ("Re: BUG 4.2.2: xl cd-insert corrupts xenstore state"): >> Turns out I already fixed this once! >> >> commit c3556e2a1aee3c9b7dda5d57e85e8867fff1b9da >> Author: George Dunlap <george.dunlap@eu.citrix.com> >> Commit: George Dunlap <george.dunlap@eu.citrix.com> > Here''s the backport, which was nearly trivial. > > I''ll push this to 4.2 staging unless someone objevts.FYI, I''m pretty sure the effect this will have is to change the failure I was seeing in 4.2 (xl crashing) to the failure Fabio and I were seeing in 4.3 with cd-eject and cd-insert. I think the situation without this patch is: * cd-eject messes up xenstore, causes future block commands to crash * cd-insert works With this patch, I think what happen sis: * cd-eject fails but does not mess up xenstore * cd-insert also fails It should be noted that this is only true *if blktap is available*; if blktap is not available, libxl uses qdev and everything works fine. (And I should emphasize, this is prediction based on my recent experience with unstable, and hasn''t been verified by testing.) The problem actually turns out to be that if blktap is available, that libxl will choose it for cdroms, but the cd-insert and cd-eject stuff don''t work with blktap. I think I''m going to submit a patch for 4.3 that prevents blktap from being used for cdroms -- you may want to wait and backport them both. -George
George Dunlap writes ("Re: BUG 4.2.2: xl cd-insert corrupts xenstore
state"):> The problem actually turns out to be that if blktap is available, that
> libxl will choose it for cdroms, but the cd-insert and cd-eject stuff
> don''t work with blktap. I think I''m going to submit a
patch for 4.3
> that prevents blktap from being used for cdroms -- you may want to wait
> and backport them both.
Right, thanks.
Ian.
Ian Jackson writes ("Re: BUG 4.2.2: xl cd-insert corrupts xenstore
state"):> George Dunlap writes ("Re: BUG 4.2.2: xl cd-insert corrupts xenstore
state"):
> > Turns out I already fixed this once!
> >
> > commit c3556e2a1aee3c9b7dda5d57e85e8867fff1b9da
> > Author: George Dunlap <george.dunlap@eu.citrix.com>
> > Commit: George Dunlap <george.dunlap@eu.citrix.com>
>
> Here''s the backport, which was nearly trivial.
>
> I''ll push this to 4.2 staging unless someone objevts.
I have now done this. Sorry for the delay.
> commit 1eea2f30f79a1af817fe32c9e01673da1f31dd5f
> Author: Ian Jackson <ian.jackson@eu.citrix.com>
> Date: Wed May 1 16:56:54 2013 +0100
>
> libxl: Fix bug in libxl_cdrom_insert, make more robust against bad
xenstore data
>
> libxl_cdrom_insert was failing to initialize the backend type,
> resulting in the wrong default backend. The result was not only that
> the CD was not inserted properly, but also that some improper xenstore
> entries were created, causing further block commands to fail.
>
> This patch fixes the bug by setting the disk backend type based on the
> type of the existing device.
>
> It also makes the system more robust by checking to see that it has
> got a valid path before proceeding to write a partial xenstore entry.
>
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> Committed-by: Ian Campbell <ian.campbell@citrix.com>
> (cherry picked from commit c3556e2a1aee3c9b7dda5d57e85e8867fff1b9da)