Mailing List Archive

(adding xen-devel)

On Thu, 2011-04-28 at 14:27 +0200, Bastian Blank wrote:
> On Thu, Apr 28, 2011 at 12:28:48PM +0100, Ian Campbell wrote:
> > Perhaps if you would describe what doesn't work for you we could work to
> > fix it, but the above isn't really very helpful, is it?
>
> The first ones:
> - Silent fail if qemu-dm[1] is missing or failing. It lacks error
> checking.

Yes, this could certainly be improved.

> - Probably missing close-on-exit flags for several file handlers.

You mean close-on-exec?

the libxl interfaces for exec'ing takes care of closing file handles and
since xl is a one-shot toolstack it generally doesn't have piles of fd's
open. The issue is still worth considering and checking for correctness
though I think, especially within libxc (which has other users than xl).

Do you know of specific instances where the CLOEXEC flag is needed but
missing?

I don't think any of the above qualifies xl as so broken we shouldn't
even suggest people try it, as you started out by saying...

Ian.

> [1]: qemu 0.10 is not supportable in any way security wise.
--
Ian Campbell

I am currently transitioning to a new OpenPGP key, please see:
http://www.hellion.org.uk/key-transition-2011-04-27-2F6BCD59-to-79074FA8.txt

/* now make a new head in the exact same spot */
-- Larry Wall in cons.c from the perl source code

On Thu, Apr 28, 2011 at 03:20:58PM +0100, Ian Campbell wrote:
> On Thu, 2011-04-28 at 14:27 +0200, Bastian Blank wrote:
> > - Silent fail if qemu-dm[1] is missing or failing. It lacks error
> > checking.
> Yes, this could certainly be improved.

A hang without any feedback is the worst user-experience possible.

> > - Probably missing close-on-exit flags for several file handlers.
> You mean close-on-exec?

Yes.

> the libxl interfaces for exec'ing takes care of closing file handles and

It tries to, but it can't ever succeed (there is no practical upper
limit for the fd value). Usually this is used to _hide_ other errors.

> Do you know of specific instances where the CLOEXEC flag is needed but
> missing?

Not right now. But because of the hiding properties it is not possible
to detect without changes anyway.

> I don't think any of the above qualifies xl as so broken we shouldn't
> even suggest people try it, as you started out by saying...

Failing without feedback and only running in an environment not
security supportable is broken.

Bastian

--
Without followers, evil cannot spread.
-- Spock, "And The Children Shall Lead", stardate 5029.5

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Thu, Apr 28, 2011 at 02:27:19PM +0200, Bastian Blank wrote:
> The first ones:
- It defaults to use qemu not when it is needed but when blktap is not
loaded.

Bastian

--
The best diplomat I know is a fully activated phaser bank.
-- Scotty

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Sat, Apr 30, 2011 at 03:09:44PM +0200, Bastian Blank wrote:
> On Thu, Apr 28, 2011 at 02:27:19PM +0200, Bastian Blank wrote:
> > The first ones:
> - It defaults to use qemu not when it is needed but when blktap is not
> loaded.

Okay, fixed in xen-unstable. I added a backport to the package.

Bastian

--
We Klingons believe as you do -- the sick should die. Only the strong
should live.
-- Kras, "Friday's Child", stardate 3497.2

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 2011-04-29 at 12:08 +0100, Bastian Blank wrote:
> On Thu, Apr 28, 2011 at 03:20:58PM +0100, Ian Campbell wrote:
> > On Thu, 2011-04-28 at 14:27 +0200, Bastian Blank wrote:
> > > - Silent fail if qemu-dm[1] is missing or failing. It lacks error
> > > checking.
> > Yes, this could certainly be improved.
>
> A hang without any feedback is the worst user-experience possible.

On my systems I get, after ~10s:
libxl: error: libxl_device.c:475:libxl__wait_for_device_model Device Model not ready
xl: fatal error: libxl_create.c:515, rc=-1: libxl__confirm_device_model_startup
libxl: debug: libxl_dm.c:890:libxl__destroy_device_model Device Model already exited

The following makes the obvious qemu-dm not present / not executable
case fail immediately by adding an access(..., X_OK) check, which is
something of an improvement.

This still leaves the delay for other cases, e.g. immediate failure due
to a missing library, bad parameters etc. I think with a little bit of
refactoring libxl__wait_for_device_model() could incorporate the
necessary check for child exit without simply waiting for the entire
delay. I'll take a closer look at this and the other issues you reported
tomorrow.

Ian.

8<------------------------------
libxl: check that device model binary is executable.

This causes us to fail more quickly in more obvious failure case of not
having the right binary installed.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

--- a/tools/libxl/libxl_dm.c Tue May 03 16:53:22 2011 +0100
+++ b/tools/libxl/libxl_dm.c Tue May 03 17:34:19 2011 +0100
@@ -762,7 +762,12 @@ int libxl__create_device_model(libxl__gc
rc = ERROR_FAIL;
goto out;
}
-
+ if (access(dm, X_OK) < 0) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "device model %s is not executable", dm);
+ rc = ERROR_FAIL;
+ goto out;
+ }
args = libxl__build_device_model_args(gc, dm, info, disks, num_disks,
vifs, num_vifs);
if (!args) {


--
Ian Campbell
Current Noise: Karma To Burn - Mt. Penetrator

Winning isn't everything, but losing isn't anything.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel