Call hotplug scripts from libxl

# HG changeset patch
# User Roger Pau Monne<[email protected]> # Date 1317300282 -7200
# Node ID 47005b3da245e2be052b614c02ed212d3d2cecbd # Parent a422e2a4451e16dc791b293f41966b842fa4781d
xenbackendd: fix incorrect usage of pidfile

Fix xenbackendd ignoring the pidfile passed through the command line.

Signed-off-by Roger Pau Monne<[email protected]>

diff -r a422e2a4451e -r 47005b3da245 tools/xenbackendd/xenbackendd.c

tools/xenbackendd/xenbackendd.c +1 -1

@@ -169,7 +169,7 @@ main(int argc, char * const argv[])
  			log_file = optarg;
  			break;
  		case 'p':
-			pidfile = pidfile;
+			pidfile = optarg;
  		case 's':
  			vbd_script = optarg;
  			break;

I've just realized that there's a drawback with this implementation, domains that are closed from inside the VM (a typical "shutdown -h now") do not trigger the launch of hotplug scripts, since libxl is not running and is not aware that the domain has powered off.

Don't know
the best way to solve this issue, that doesn't take us back to having a separate daemon that monitors xenstore states (or udev on Linux).

Regards, Roger.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

By default xl daemonises itself and monitors the domain waiting for it to exit, at which point it should perform the necessary cleanup (this is the call to libxl_domain_destroy in handle_domain_death).

If you ask xl to not do this (using the -e or -F options) then you are expected to call xl destroy yourself.

The call to libxl_domain_destroy calls libxl__devices_destroy which tears down the backends. Unfortunately this is done in a slightly generic way but my recent series should have laid the ground work for doing this in a more device-specific way.

I have a feeling that libxl__device_destroy should really consist of a sequence of libxl_device_FOO_list+foreach(thing) libxl_device_FOO_remove calls rather than the current mechanism. i.e. be really explicit about what it is tearing down.

Ian.

2011/10/4 Ian Campbell <[email protected]>:

Quoted 3 lines

By default xl daemonises itself and monitors the domain waiting for it to exit, at which point it should perform the necessary cleanup (this is the call to libxl_domain_destroy in handle_domain_death).

libxl daemonises itself if you call "xl shutdown <domain>", but if the domain shuts down from inside (doing a "halt"), xl is never called, and therefore hotplug scripts are not executed.

Quoted 15 lines

If you ask xl to not do this (using the -e or -F options) then you are expected to call xl destroy yourself.

The call to libxl_domain_destroy calls libxl__devices_destroy which tears down the backends. Unfortunately this is done in a slightly generic way but my recent series should have laid the ground work for doing this in a more device-specific way.

I have a feeling that libxl__device_destroy should really consist of a sequence of libxl_device_FOO_list+foreach(thing) libxl_device_FOO_remove calls rather than the current mechanism. i.e. be really explicit about what it is tearing down.

Ian.

I don't think "xl shutdown" daemonises ever.

"xl create" daemonises itself and waits for the guest to die. It should be detecting the guest shutdown no matter if it is triggered internally or externally. That's not to say it's not buggy though...

xenbackendd: fix incorrect usage of pidfile

Fix xenbackendd ignoring the pidfile passed through the command line.

Signed-off-by Roger Pau Monne <[email protected]>

Sometimes PyGrub freezed when trying to create a domain without the console attached (without "-c"). This patch adds the "-q" option to PyGrub when "-c" is not specified at creation time. PyGrub freezed trying to set terminal attributes (like reset_prog_mode or nocbreak).

I think "-c" shouldn't affect the functionality of the domain, just whether the console is initially connected.  Perhaps the console will be connected somewhere else.

OTOH I think it would be fine to have an option (libxl config parameter, I guess) to make pygrub run noninteractively.

Quoted 6 lines

I think "-c" shouldn't affect the functionality of the domain, just whether the console is initially connected. Perhaps the console will be connected somewhere else.

OTOH I think it would be fine to have an option (libxl config parameter, I guess) to make pygrub run noninteractively.

I've digged a little more into this issue, and found out that NetBSD ptys have a 1024 buffer limit, and when this limit is hit, the select

from libxl_bootloader:211 blocks and no more data is writen to xenconsole_fd and no more data is read from bootloader_fd (because bootloader_buf contains data),

which of course blocks the execution of
pygrub. Since the xenconsoled_fd pty is not monitored by xenconsole, nobody is draining data from it. I don't know the best way to solve this, but it seems like somebody should read the data from xenconsoled_fd and dispose it if there is no console attached.

1K? More of a workaround than a solution I suppose.

Quoted 9 lines · Roger Pau Monné

Quoted 6 lines

I think "-c" shouldn't affect the functionality of the domain, just whether the console is initially connected. Perhaps the console will be connected somewhere else.

OTOH I think it would be fine to have an option (libxl config parameter, I guess) to make pygrub run noninteractively.

I've digged a little more into this issue, and found out that NetBSD ptys have a 1024 buffer limit, and when this limit is hit, the select

                  ^byte ?

Quoted 3 lines

from libxl_bootloader:211 blocks and no more data is writen to xenconsole_fd and no more data is read from bootloader_fd (because bootloader_buf contains data),

IIRC I made that last condition like that because it simplified things because I could avoid worrying about wrapping the consumer round in the buffer, or something. However if that scheme doesn't work for BSD we could go the more complex route, assuming it would help.

This could well be a problem on Linux too but we are just avoiding it due to the larger buffers. I wonder if I can artificially reduce the buffer size on Linux...

Quoted 5 lines

which of course blocks the execution of
pygrub. Since the xenconsoled_fd pty is not monitored by xenconsole, nobody is draining data from it. I don't know the best way to solve this, but it seems like somebody should read the data from xenconsoled_fd and dispose it if there is no console attached.

Yes. One option could be to add a timeout to the select and throwaway data if necessary. I think xenconsoled does this for real guest consoles too if no one is connected so that behaviour would at least be consistent. Or maybe xenconsoled should be doing this for this pty like it does for guests?

Might another option be to increase the size of the buffer in libxl to

Quoted 1 line

1K? More of a workaround than a solution I suppose.

Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

I've digged a little more into this issue, and found out that NetBSD ptys have a 1024 buffer limit, and when this limit is hit, the select from libxl_bootloader:211 blocks and no more data is writen to xenconsole_fd and no more data is read from bootloader_fd (because bootloader_buf contains data), which of course blocks the execution of pygrub. Since the xenconsoled_fd pty is not monitored by xenconsole, nobody is draining data from it. I don't know the best way to solve this, but it seems like somebody should read the data from xenconsoled_fd and dispose it if there is no console attached.

That sounds unfortunate.  I agree with you.

Quoted 1 line

That sounds unfortunate. I agree with you.

Well Ian Campbell proposed to use a timeout in select and drop data when the timeout is hit, that what I've implemented in

libxl: drop bootloader output if xenconsole file descriptor is not available for writing

The patch is quite simple and solves the problem, I don't think it interferes with normal boot process, since a 1s timeout is quite high for data to be delivered from xenconsoled_fd to the user if the console is attached.

The other option would be to modify xenconsole
daemon, but I think that's far too complicated for a problem that can be solved easily.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Quoted 5 lines

Quoted 1 line

That sounds unfortunate.  I agree with you.

Well Ian Campbell proposed to use a timeout in select and drop data when the timeout is hit, that what I've implemented in

Quoted 7 lines

libxl: drop bootloader output if xenconsole file descriptor is not available for writing

The patch is quite simple and solves the problem, I don't think it interferes with normal boot process, since a 1s timeout is quite high for data to be delivered from xenconsoled_fd to the user if the console is attached.

The case where a user types
       xl create /a/domain
       xl console adomain

might have a reasonably long delay in it before the console is connected. Increasing the libxl buffer size would mitigate that but you'd still have the problem of not reading from an fd if the cons is != 0.

Can you print out the actual values of the various producers and consumers at the point of the hang? (maybe track total bytes too for debug?)

Perhaps where we have:
       if (xenconsoled_prod == xenconsoled_cons) xenconsoled_prod = xenconsoled_cons = 0;
       if (bootloader_prod == bootloader_cons) bootloader_prod = bootloader_cons = 0;
We could also add some suitable memmoves and associated prod/cons manipulation, such that the unconsumed data is always pulled to the head of the buffer. That might be a reasonably simple way to improve things and allow larger buffers to be used? More often than not the memmove won't be getting called since either XXX_prod == XXX_cons or XXX_cons is already 0, IOW having a partially consumed buffer would be unusual?

Once we have a larger buffer which we always try to fill discarding data after a timeout when the buffer is full won't be so critical.

2011/10/11 Ian Campbell <[email protected]>:

Quoted 18 lines · Roger Pau Monné

Quoted 5 lines

Quoted 1 line

That sounds unfortunate. I agree with you.

Well Ian Campbell proposed to use a timeout in select and drop data when the timeout is hit, that what I've implemented in

Quoted 7 lines

libxl: drop bootloader output if xenconsole file descriptor is not available for writing

The patch is quite simple and solves the problem, I don't think it interferes with normal boot process, since a 1s timeout is quite high for data to be delivered from xenconsoled_fd to the user if the console is attached.

The case where a user types
xl create /a/domain
xl console adomain

Quoted 8 lines

might have a reasonably long delay in it before the console is connected. Increasing the libxl buffer size would mitigate that but you'd still have the problem of not reading from an fd if the cons is != 0.

Can you print out the actual values of the various producers and consumers at the point of the hang? (maybe track total bytes too for debug?)

The process gets stuck after writing 1022 bytes to xenconsole_fd, and the buffer is 24 consumed 48 produced (although that varies, sometimes I get ~700 produced and ~20 consumed, but the total written bits is 1022 always).

Quoted 6 lines

Perhaps where we have:

       if (xenconsoled_prod == xenconsoled_cons)
           xenconsoled_prod = xenconsoled_cons = 0;
       if (bootloader_prod == bootloader_cons)
           bootloader_prod = bootloader_cons = 0;

Quoted 6 lines

We could also add some suitable memmoves and associated prod/cons manipulation, such that the unconsumed data is always pulled to the head of the buffer. That might be a reasonably simple way to improve things and allow larger buffers to be used? More often than not the memmove won't be getting called since either XXX_prod == XXX_cons or XXX_cons is already 0, IOW having a partially consumed buffer would be unusual?

I don't really like moving memory around, since it's slow, but I think implementing a circular buffer would really complicate things.

I will
post a new patch that implements the following:

1. Increase buffer.
2. Use memmove to move memory to the head of the buffer and fill it until the buffer is full.
3. On timeout and full buffer, drop data from the head of the buffer (older data) and fill it with new data (move the buffer and append new data at the end).

Do you thing this should be applied to both xenconsoled and bootloader? since the only one giving problems is xenconsoled.

Quoted 2 lines

Once we have a larger buffer which we always try to fill discarding data after a timeout when the buffer is full won't be so critical.

# HG changeset patch
# User Roger Pau Monne <[email protected]> # Date 1317386335 -7200
# Node ID 156626fef95b36184ad44dfcb049bae2545435f0 # Parent 31b3caad18ac904a242c5bd1ea2bb0e574cd0453
libxl: create pci backend only when there are pci devices.

Creating empty pci entries made Linux DomUs under NetBSD Dom0 wait a very long time for devices to initialize during kernel boot.

Quoted 7 lines

# HG changeset patch
# User Roger Pau Monne <[email protected]> # Date 1317386335 -7200
# Node ID 156626fef95b36184ad44dfcb049bae2545435f0 # Parent  31b3caad18ac904a242c5bd1ea2bb0e574cd0453
libxl: create pci backend only when there are pci devices.

Quoted 2 lines

Creating empty pci entries made Linux DomUs under NetBSD Dom0 wait a very long time for devices to initialize during kernel boot.

I think since this change we need the following. This applies after my big libxl API series:

Ian.

8<------------------------------------------------------

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318943709 -3600
# Node ID e6af32ecb4483457d5ad9a7b674c18aaf20cc085
# Parent  12ac478d1b56f0ff4d3242138b780c551a69f107
libxl: libxl__device_pci_destroy_all should succeed if there is no PCI bus on the domain

Since 23915:4c4b72c94bac "" it is possible that no bus exists and this is not an error. Removes the following spurious warning when destroying a PV domain with no PCI devices:
   libxl: error: libxl.c:759:libxl_domain_destroy: pci shutdown failed for domid 1005

Signed-off-by Ian Campbell <[email protected]>

diff -r 12ac478d1b56 -r e6af32ecb448 tools/libxl/libxl_pci.c

tools/libxl/libxl_pci.c   Tue Oct 18 14:15:09 2011 +0100 renamed from tools/libxl/libxl_pci.c   Tue Oct 18 13:36:43 2011 +0100 +1 -1

@@ -1098,7 +1098,7 @@ int libxl__device_pci_destroy_all(libxl_
 
     pcidevs = libxl_device_pci_list(ctx, domid, &num);
     if ( pcidevs == NULL )
-        return ERROR_FAIL;
+        return 0;
 
     for (i = 0; i < num; i++) {
         /* Force remove on shutdown since, on HVM, qemu will not always

2011/10/18 Ian Campbell <[email protected]>:

Quoted 11 lines · Roger Pau Monne

Quoted 9 lines

# HG changeset patch
# User Roger Pau Monne <[email protected]> # Date 1317386335 -7200
# Node ID 156626fef95b36184ad44dfcb049bae2545435f0 # Parent 31b3caad18ac904a242c5bd1ea2bb0e574cd0453
libxl: create pci backend only when there are pci devices.

Creating empty pci entries made Linux DomUs under NetBSD Dom0 wait a very long time for devices to initialize during kernel boot.

Quoted 2 lines

I think since this change we need the following. This applies after my big libxl API series:

Yes, this fixes the issue related to the error message "libxl: error:
libxl.c:759:libxl_domain_destroy: pci shutdown failed for domid". I don't know if I can do this, but anyway:

Acked-by Roger Pau Monne <[email protected]>

Quoted 35 lines

Ian.

8<------------------------------------------------------

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318943709 -3600
# Node ID e6af32ecb4483457d5ad9a7b674c18aaf20cc085
# Parent 12ac478d1b56f0ff4d3242138b780c551a69f107
libxl: libxl__device_pci_destroy_all should succeed if there is no PCI bus on the domain

Since 23915:4c4b72c94bac "" it is possible that no bus exists and this is not an error. Removes the following spurious warning when destroying a PV domain with no PCI devices:
libxl: error: libxl.c:759:libxl_domain_destroy: pci shutdown failed for domid 1005

Signed-off-by Ian Campbell <[email protected]>

diff -r 12ac478d1b56 -r e6af32ecb448 tools/libxl/libxl_pci.c

tools/libxl/libxl_pci.c Tue Oct 18 14:15:09 2011 +0100 renamed from tools/libxl/libxl_pci.c Tue Oct 18 13:36:43 2011 +0100 +1 -1

@@ -1098,7 +1098,7 @@ int libxl__device_pci_destroy_all(libxl_
 
    pcidevs = libxl_device_pci_list(ctx, domid, &num);
    if ( pcidevs == NULL )
-        return ERROR_FAIL;
+        return 0;
 
    for (i = 0; i < num; i++) {
        /* Force remove on shutdown since, on HVM, qemu will not always

Please remember that this patch fixing the removal of PCI entries has not been applied along with the "libxl: rationalise libxl_device_* APIs" series.

libxl: libxl__device_pci_destroy_all should succeed if there is no PCI bus on the domain

Acked-by Roger Pau Monne <[email protected]>