Re: [Libguestfs] [PATCH v6 11/41] utils: Rename ‘guestfs-internal-frontend.h’ to ‘utils.h’.

[Richard W.M. Jones]

On Fri, Jun 16, 2017 at 03:42:35PM +0200, Pino Toscano wrote:
> NACK to utils.h -- in the past I've seen a couple of libraries
> installing public includes as utils.h. They have been fixed, but
> I'd like to avoid conflicts between an installed header and a project
> header.

Can we prevent incorrect installation (which I agree could be a huge
problem) in some other way?  Maybe it can be done with an
install-*-hook.  I will see ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v6 10/41] mllib, v2v: Split out OCaml utils bindings ‘common/mlutils’.

[Richard W.M. Jones]

On Fri, Jun 16, 2017 at 03:38:35PM +0200, Pino Toscano wrote:
> On Thursday, 15 June 2017 19:06:00 CEST Richard W.M. Jones wrote:
> > Create a module ‘C_utils’ containing functions like ‘drive_name’ and
> > ‘shell_unquote’ which come from the C utilities.
> > 
> > The new directory ‘common/mlutils’ also contains the ‘Unix_utils’
> > wrappers around POSIX functions missing from the OCaml stdlib.
> > ---
> 
> I fear we are spreading the code among too many helper libraries...
> Why not just add these small bindings to Common_utils directly?

It may not be well explained in the commit messages, but the reason is
because the generator is pure OCaml (and bytecode too) so it can't
link to C functions.  So there's a pure OCaml library (Std_utils), and
this library which the daemon can use.

It's actually not split up for fun (which it certainly was not!)  but
because different things need different subsets.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v6 05/41] utils: Split out cleanups into common/cleanups.

[Richard W.M. Jones]

On Fri, Jun 16, 2017 at 03:24:55PM +0200, Pino Toscano wrote:
> On Thursday, 15 June 2017 19:05:55 CEST Richard W.M. Jones wrote:
> > Those cleanups which only depend on libc, gnulib or libxml2 are split
> > out into a separate common/cleanups directory.
> > ---
> 
> IMHO a single cleanups.c source should be enough, otherwise it's overly
> split...

I think you do need to split it.  The reason is that if the program
uses libcleanups.la but doesn't link to (eg) libxml2 then the link
will fail.  We could either force everything to link unnecessarily to
libxml2 or we can split the object files so that the libxml2
dependency is never pulled in if the main program doesn't use it.

And the same applies (but a bit less) to gnulib.  I'm not sure
anything doesn't link to gnulib though, and probably everything should
(except examples but they don't use cleanups).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v6 11/41] utils: Rename ‘guestfs-internal-frontend.h’ to ‘utils.h’.

[Pino Toscano]

NACK to utils.h -- in the past I've seen a couple of libraries
installing public includes as utils.h. They have been fixed, but
I'd like to avoid conflicts between an installed header and a project
header.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v6 10/41] mllib, v2v: Split out OCaml utils bindings ‘common/mlutils’.

[Pino Toscano]

On Thursday, 15 June 2017 19:06:00 CEST Richard W.M. Jones wrote:
> Create a module ‘C_utils’ containing functions like ‘drive_name’ and
> ‘shell_unquote’ which come from the C utilities.
> 
> The new directory ‘common/mlutils’ also contains the ‘Unix_utils’
> wrappers around POSIX functions missing from the OCaml stdlib.
> ---

I fear we are spreading the code among too many helper libraries...
Why not just add these small bindings to Common_utils directly?

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v6 05/41] utils: Split out cleanups into common/cleanups.

[Pino Toscano]

On Thursday, 15 June 2017 19:05:55 CEST Richard W.M. Jones wrote:
> Those cleanups which only depend on libc, gnulib or libxml2 are split
> out into a separate common/cleanups directory.
> ---

IMHO a single cleanups.c source should be enough, otherwise it's overly
split...

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v6 04/41] mllib: Split ‘Common_utils’ into ‘Std_utils’ + ‘Common_utils’.

[Pino Toscano]

On Thursday, 15 June 2017 19:05:54 CEST Richard W.M. Jones wrote:
> The new module ‘Std_utils’ contains only functions which are pure
> OCaml and depend only on the OCaml stdlib.  Therefore these functions
> may be used by the generator.

Hm can we please use a better name than Std_utils? Otherwise there's
a bit of confusion between two generic names such as Std_utils and
Common_utils.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH] inspection: Deprecate APIs and remove support for inspecting installer CDs.

[Richard W.M. Jones]

This just duplicated libosinfo information, and because it was never
tested it didn't work most of the time.
---
 docs/C_SOURCE_FILES|   2 -
 generator/actions_inspection.ml|  67 ---
 generator/actions_inspection_deprecated.ml |  61 +++
 inspector/Makefile.am  |  11 +-
 inspector/example-debian-netinst-cd.xml|  23 -
 inspector/example-debian.xml   |   1 -
 inspector/example-fedora-dvd.xml   |  23 -
 inspector/example-fedora-netinst-cd.xml|  21 -
 inspector/example-fedora.xml   |   1 -
 inspector/example-rhel-6-dvd.xml   |  23 -
 inspector/example-rhel-6-netinst-cd.xml|  21 -
 inspector/example-rhel-6.xml   |   1 -
 inspector/example-ubuntu-live-cd.xml   |  23 -
 inspector/example-ubuntu.xml   |   1 -
 inspector/example-windows-2003-x64-cd.xml  |  24 --
 inspector/example-windows-2003-x86-cd.xml  |  24 --
 inspector/example-windows-xp-cd.xml|  24 --
 inspector/example-windows.xml  |   1 -
 inspector/expected-archlinux.img.xml   |   1 -
 inspector/expected-coreos.img.xml  |   1 -
 inspector/expected-debian.img.xml  |   1 -
 inspector/expected-fedora.img.xml  |   1 -
 inspector/expected-ubuntu.img.xml  |   1 -
 inspector/expected-windows.img.xml |   1 -
 inspector/inspector.c  |  31 +-
 inspector/virt-inspector.pod   |  22 -
 inspector/virt-inspector.rng   |  15 -
 lib/Makefile.am|   3 -
 lib/guestfs-internal.h |  31 --
 lib/guestfs.pod|   9 -
 lib/inspect-fs-cd.c| 606 --
 lib/inspect-fs.c   |  40 --
 lib/osinfo.c   | 655 -
 33 files changed, 63 insertions(+), 1707 deletions(-)

diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES
index 15abec124..dc8c052dd 100644
--- a/docs/C_SOURCE_FILES
+++ b/docs/C_SOURCE_FILES
@@ -296,7 +296,6 @@ lib/guid.c
 lib/handle.c
 lib/info.c
 lib/inspect-apps.c
-lib/inspect-fs-cd.c
 lib/inspect-fs-unix.c
 lib/inspect-fs-windows.c
 lib/inspect-fs.c
@@ -315,7 +314,6 @@ lib/listfs.c
 lib/lpj.c
 lib/match.c
 lib/mountable.c
-lib/osinfo.c
 lib/private-data.c
 lib/proto.c
 lib/qemu.c
diff --git a/generator/actions_inspection.ml b/generator/actions_inspection.ml
index b7ea5a4de..cd8b9da18 100644
--- a/generator/actions_inspection.ml
+++ b/generator/actions_inspection.ml
@@ -566,73 +566,6 @@ string C is returned.
 Please read L for more details." };
 
   { defaults with
-name = "inspect_get_format"; added = (1, 9, 4);
-style = RString (RPlainString, "format"), [String (Mountable, "root")], [];
-shortdesc = "get format of inspected operating system";
-longdesc = "\
-This returns the format of the inspected operating system.  You
-can use it to detect install images, live CDs and similar.
-
-Currently defined formats are:
-
-=over 4
-
-=item \"installed\"
-
-This is an installed operating system.
-
-=item \"installer\"
-
-The disk image being inspected is not an installed operating system,
-but a I install disk, live CD, or similar.
-
-=item \"unknown\"
-
-The format of this disk image is not known.
-
-=back
-
-Future versions of libguestfs may return other strings here.
-The caller should be prepared to handle any string.
-
-Please read L for more details." };
-
-  { defaults with
-name = "inspect_is_live"; added = (1, 9, 4);
-style = RBool "live", [String (Mountable, "root")], [];
-shortdesc = "get live flag for install disk";
-longdesc = "\
-If C returns C (this
-is an install disk), then this returns true if a live image
-was detected on the disk.
-
-Please read L for more details." };
-
-  { defaults with
-name = "inspect_is_netinst"; added = (1, 9, 4);
-style = RBool "netinst", [String (Mountable, "root")], [];
-shortdesc = "get netinst (network installer) flag for install disk";
-longdesc = "\
-If C returns C (this
-is an install disk), then this returns true if the disk is
-a network installer, ie. not a self-contained install CD but
-one which is likely to require network access to complete
-the install.
-
-Please read L for more details." };
-
-  { defaults with
-name = "inspect_is_multipart"; added = (1, 9, 4);
-style = RBool "multipart", [String (Mountable, "root")], [];
-shortdesc = "get multipart flag for install disk";
-longdesc = "\
-If C returns C (this
-is an install disk), then this returns true if the disk is
-part of a set.
-
-Please read L for more details." };
-
-  { defaults with
 name = "inspect_get_product_variant"; added = (1, 9, 13);
 style = RString (RPlainString, "variant"), [String (Mountable, "root")], 
[];
 shortdesc = "get 

Re: [Libguestfs] [PATCH 2/2] python: unicode decode handler error scheme setter

[Pino Toscano]

On Sunday, 21 May 2017 18:29:03 CEST Matteo Cafasso wrote:
> The set_decode_error_handler function allows the User to set the
> decoding error scheme to be used when non UTF8 characters are
> encountered in Python 3.

s/User/user/, and s/UTF8/UTF-8/

> The function has no effect in Python 2.
> 
> Signed-off-by: Matteo Cafasso 
> ---
>  generator/python.ml| 16 
>  python/handle.c| 18 --
>  python/t/test830RHBZ1406906.py |  6 ++
>  3 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/generator/python.ml b/generator/python.ml
> index f7c1f80bb..66bb7f27d 100644
> --- a/generator/python.ml
> +++ b/generator/python.ml
> @@ -82,6 +82,7 @@ put_handle (guestfs_h *g)
>  }
> 
>  extern void guestfs_int_py_extend_module (PyObject *module);
> +extern PyObject *guestfs_int_py_set_decode_error_handler (PyObject *self, 
> PyObject *args);
> 
>  extern PyObject *guestfs_int_py_create (PyObject *self, PyObject *args);
>  extern PyObject *guestfs_int_py_close (PyObject *self, PyObject *args);
> @@ -577,6 +578,8 @@ and generate_python_module () =
> 
>(* Table of functions. *)
>pr "static PyMethodDef methods[] = {\n";
> +  pr "  { (char *) \"set_decode_error_handler\", \n";
> +  pr "guestfs_int_py_set_decode_error_handler, METH_VARARGS, NULL },\n";

This is implemented as global for the whole module, which means
changing the behaviour for an handle changes it for all the existing
handles (and in a racy behaviour, even).  This IMHO should be a
per-handle setting.

>pr "  { (char *) \"create\", guestfs_int_py_create, METH_VARARGS, NULL 
> },\n";
>pr "  { (char *) \"close\", guestfs_int_py_close, METH_VARARGS, NULL },\n";
>pr "  { (char *) \"set_event_callback\",\n";
> @@ -728,6 +731,19 @@ class ClosedHandle(ValueError):
>  pass
> 
> 
> +def set_decode_error_handler(handler):

'handler' usually is a function/callback, while in this case is a
behaviour/mode, so I'd use a different naming.

> +\"\"\"Set the error handling scheme to use for the handling
> +of decoding errors.
> +The default is 'strict' meaning that decoding errors raise a
> +UnicodeDecodeError.
> +
> +The other possible value is 'surrogateescape', see PEP383 for reference.
> +
> +Return the previous error handler.
> +\"\"\"
> +return libguestfsmod.set_decode_error_handler(handler)
> +
> +
>  class GuestFS(object):
>  \"\"\"Instances of this class are libguestfs API handles.\"\"\"
> 
> diff --git a/python/handle.c b/python/handle.c
> index 52c36f1d2..b665bb899 100644
> --- a/python/handle.c
> +++ b/python/handle.c
> @@ -35,6 +35,8 @@
> 
>  #include "actions.h"
> 
> +static const char *decode_error_handler = "strict";
> +
>  static PyObject **get_all_event_callbacks (guestfs_h *g, size_t *len_rtn);
> 
>  void
> @@ -45,6 +47,17 @@ guestfs_int_py_extend_module (PyObject *module)
>  }
> 
>  PyObject *
> +guestfs_int_py_set_decode_error_handler (PyObject *self, PyObject *args)
> +{
> +  const char *previous_handler = decode_error_handler;
> +
> +  if (!PyArg_ParseTuple (args, (char *) "s:set_decode_error_handler", 
> _error_handler))
> +return NULL;

I really doubt "decode_error_handler" will hold a valid pointer after
guestfs_int_py_set_decode_error_handler is done (and the args PyObject
is disposed.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs