This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] [PATCH] libxl: blktap2 portiblity fixes

Christoph Egger writes ("Re: [Xen-devel] [PATCH] libxl: blktap2 portiblity 
> Can you use the c/s numbers, please?

Changeset numbers are not guaranteed to be meaningful outside a
particular tree, particularly in the presence of merges.

> It was not necessary to backout c/s 21834 as this wasn't the root cause.

I think by 21834 you mean 24277e3237ca.  That change was entirely
wrong and you even retracted it yourself!  I see that your patch
doesn't actually reinstate it.

Looking at this patch it's difficult to review and test and there are
some things I would like to see improved.  Can I ask you to try to
split the patch up into separate pieces ?

At the very least, please separate out the following:
  * Adding new interfaces, ie introucing new functions and putting
    code into those functions and replacing it at the original site
    with a call;
  * Const-correctness
  * Code motion between files, and creation of libxl_linux.c;
  * Provide libxl_netbsd.c and associated Makefile changes to
    disable blktap2 on netbsd
  * #include portability fixes for openpty, pty.h etc.

> diff -r 2b768d52bc7f tools/libxl/libxl_bootloader.c
> --- a/tools/libxl/libxl_bootloader.c  Sun Jul 25 22:20:47 2010 +0100
> +++ b/tools/libxl/libxl_bootloader.c  Mon Jul 26 12:41:02 2010 +0200
> @@ -15,9 +15,16 @@
>  #include "libxl_osdeps.h"
>  #include <string.h>
> -#include <pty.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> +#include <termios.h>
> +#if defined(__NetBSD__) || defined(__OpenBSD__)
> +#include <util.h>
> +#elif defined(__linux__)
> +#include <pty.h>
> +#elif defined(__sun__)
> +#include <stropts.h>
> +#endif

This should be done by moving the relevant #includes to osdep.h, where
all this kind of thing should be done.

> -int device_physdisk_major_minor(char *physpath, int *major, int *minor)
> +int device_physdisk_major_minor(const char *physpath, int *major, int *minor)

This change should be separated out.


Xen-devel mailing list