[PATCH 0/4] Fix to libxl migration v2 issue blocking OSSTest

[PATCH 0/4] Fix to libxl migration v2 issue blocking OSSTest

[Andrew Cooper]

And three improvements to debugging.

Note that there is still a bug in libxl__toolstack_save() which
valgrind identified, but I do not wish to block this bugfix on that

Andrew Cooper (4):
  tools/libxc: Identify the path of the kernel image which cannot be
    found
  tools/libxl: Log the subject fd in datacopier messages
  tools/libxl: Identify copywhat in stream v2 datacopiers
  tools/libxl: Initialise the fd of the unused half of a datacopier

 tools/libxc/xg_private.c         |    2 +-
 tools/libxl/libxl_aoutils.c      |   24 ++++++++++++------------
 tools/libxl/libxl_stream_read.c  |   19 +++++++++++--------
 tools/libxl/libxl_stream_write.c |    5 ++++-
 4 files changed, 28 insertions(+), 22 deletions(-)

-- 
1.7.10.4

[PATCH 1/4] tools/libxc: Identify the path of the kernel image which cannot be found

[Andrew Cooper]

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>

---
This is rather quicker than using strace to find out that I had
forgotten to ./configure --prefix=/usr
---
 tools/libxc/xg_private.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxc/xg_private.c b/tools/libxc/xg_private.c
index c52cb44..507b205 100644
--- a/tools/libxc/xg_private.c
+++ b/tools/libxc/xg_private.c
@@ -37,7 +37,7 @@ char *xc_read_image(xc_interface *xch,
 
     if ( (kernel_fd = open(filename, O_RDONLY)) < 0 )
     {
-        PERROR("Could not open kernel image");
+        PERROR("Could not open kernel image '%s'", filename);
         goto out;
     }
 
-- 
1.7.10.4

[PATCH 2/4] tools/libxl: Log the subject fd in datacopier messages

[Andrew Cooper]

This is a substantial aid to debugging

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_aoutils.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 9087c23..d5fbc4d 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -188,14 +188,13 @@ void libxl__datacopier_prefixdata(libxl__egc *egc, libxl__datacopier_state *dc,
 
 static int datacopier_pollhup_handled(libxl__egc *egc,
                                       libxl__datacopier_state *dc,
-                                      short revents, int onwrite)
+                                      int fd, short revents, int onwrite)
 {
     STATE_AO_GC(dc->ao);
 
     if (dc->callback_pollhup && (revents & POLLHUP)) {
-        LOG(DEBUG, "received POLLHUP on %s during copy of %s",
-            onwrite ? dc->writewhat : dc->readwhat,
-            dc->copywhat);
+        LOG(DEBUG, "received POLLHUP on fd %d: %s during copy of %s",
+            fd, onwrite ? dc->writewhat : dc->readwhat, dc->copywhat);
         libxl__datacopier_kill(dc);
         dc->callback_pollhup(egc, dc, ERROR_FAIL, onwrite, -1);
         return 1;
@@ -217,13 +216,13 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
     libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, toread);
     STATE_AO_GC(dc->ao);
 
-    if (datacopier_pollhup_handled(egc, dc, revents, 0))
+    if (datacopier_pollhup_handled(egc, dc, fd, revents, 0))
         return;
 
     if (revents & ~(POLLIN|POLLHUP)) {
-        LOG(ERROR,
-            "unexpected poll event 0x%x (expected POLLIN and/or POLLHUP)"
-            " on %s during copy of %s", revents, dc->readwhat, dc->copywhat);
+        LOG(ERROR, "unexpected poll event 0x%x on fd %d (expected POLLIN "
+            "and/or POLLHUP) reading %s during copy of %s",
+            revents, fd, dc->readwhat, dc->copywhat);
         datacopier_callback(egc, dc, ERROR_FAIL, -1, EIO);
         return;
     }
@@ -286,7 +285,7 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
                     LIBXL__EVENT_DISASTER(egc,
      "unexpected failure polling fd for datacopier eof hup check",
                                   errno, 0);
-                if (datacopier_pollhup_handled(egc, dc, hupchk.revents, 0))
+                if (datacopier_pollhup_handled(egc, dc, fd, hupchk.revents, 0))
                     return;
             }
             libxl__ev_fd_deregister(gc, &dc->toread);
@@ -320,12 +319,13 @@ static void datacopier_writable(libxl__egc *egc, libxl__ev_fd *ev,
     libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, towrite);
     STATE_AO_GC(dc->ao);
 
-    if (datacopier_pollhup_handled(egc, dc, revents, 1))
+    if (datacopier_pollhup_handled(egc, dc, fd, revents, 1))
         return;
 
     if (revents & ~POLLOUT) {
-        LOG(ERROR, "unexpected poll event 0x%x (should be POLLOUT)"
-            " on %s during copy of %s", revents, dc->writewhat, dc->copywhat);
+        LOG(ERROR, "unexpected poll event 0x%x on fd %d (should be POLLOUT)"
+            " writing %s during copy of %s",
+            revents, fd, dc->writewhat, dc->copywhat);
         datacopier_callback(egc, dc, ERROR_FAIL, -1, EIO);
         return;
     }
-- 
1.7.10.4

[PATCH 3/4] tools/libxl: Identify copywhat in stream v2 datacopiers

[Andrew Cooper]

This is an aid to debugging

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_stream_read.c  |   18 ++++++++++--------
 tools/libxl/libxl_stream_write.c |    4 +++-
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index 2d17403..3e1cd2a 100644
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -223,9 +223,10 @@ void libxl__stream_read_start(libxl__egc *egc,
     }
     /* stream->fd is now a v2 stream. */
 
-    dc->ao      = stream->ao;
-    dc->readfd  = stream->fd;
-    dc->writefd = -1;
+    dc->ao       = stream->ao;
+    dc->copywhat = "restore v2 stream";
+    dc->readfd   = stream->fd;
+    dc->writefd  = -1;
 
     /* Start reading the stream header. */
     rc = setup_read(stream, "stream header",
@@ -606,11 +607,12 @@ static void write_emulator_blob(libxl__egc *egc,
     }
 
     FILLZERO(*dc);
-    dc->ao = stream->ao;
-    dc->writewhat = "qemu save file";
-    dc->writefd = writefd;
-    dc->maxsz = -1;
-    dc->callback = write_emulator_done;
+    dc->ao         = stream->ao;
+    dc->writewhat  = "qemu save file";
+    dc->copywhat   = "restore v2 stream";
+    dc->writefd    = writefd;
+    dc->maxsz      = -1;
+    dc->callback   = write_emulator_done;
 
     rc = libxl__datacopier_start(dc);
     if (rc)
diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
index 944a87b..94f681b 100644
--- a/tools/libxl/libxl_stream_write.c
+++ b/tools/libxl/libxl_stream_write.c
@@ -180,7 +180,8 @@ void libxl__stream_write_start(libxl__egc *egc,
 
     dc->ao        = ao;
     dc->readfd    = -1;
-    dc->writewhat = "save/migration stream";
+    dc->writewhat = "stream header";
+    dc->copywhat  = "save v2 stream";
     dc->writefd   = stream->fd;
     dc->maxsz     = -1;
     dc->callback  = stream_header_done;
@@ -386,6 +387,7 @@ static void write_emulator_record(libxl__egc *egc,
     FILLZERO(*dc);
     dc->ao            = stream->ao;
     dc->readwhat      = "qemu save file";
+    dc->copywhat      = "save v2 stream";
     dc->readfd        = readfd;
     dc->maxsz         = -1;
     dc->readbuf       = stream->emu_body + sizeof(*ehdr);
-- 
1.7.10.4

[PATCH 4/4] tools/libxl: Initialise the fd of the unused half of a datacopier

[Andrew Cooper]

This bug causes a spurious failure if stdin happens to be an
appropriately readable/writeable pipe which receives a POLLHUP

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_stream_read.c  |    1 +
 tools/libxl/libxl_stream_write.c |    1 +
 2 files changed, 2 insertions(+)

diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index 3e1cd2a..32a3551 100644
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -611,6 +611,7 @@ static void write_emulator_blob(libxl__egc *egc,
     dc->writewhat  = "qemu save file";
     dc->copywhat   = "restore v2 stream";
     dc->writefd    = writefd;
+    dc->readfd     = -1;
     dc->maxsz      = -1;
     dc->callback   = write_emulator_done;
 
diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
index 94f681b..47f71c0 100644
--- a/tools/libxl/libxl_stream_write.c
+++ b/tools/libxl/libxl_stream_write.c
@@ -389,6 +389,7 @@ static void write_emulator_record(libxl__egc *egc,
     dc->readwhat      = "qemu save file";
     dc->copywhat      = "save v2 stream";
     dc->readfd        = readfd;
+    dc->readfd        = -1;
     dc->maxsz         = -1;
     dc->readbuf       = stream->emu_body + sizeof(*ehdr);
     dc->bytes_to_read = rec->length - sizeof(*ehdr);
-- 
1.7.10.4

Re: [PATCH 1/4] tools/libxc: Identify the path of the kernel image which cannot be found

[Ian Campbell]

On Fri, 2015-07-17 at 17:51 +0100, Andrew Cooper wrote:
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH 4/4] tools/libxl: Initialise the fd of the unused half of a datacopier

[Ian Campbell]

On Fri, 2015-07-17 at 17:51 +0100, Andrew Cooper wrote:
> This bug causes a spurious failure if stdin happens to be an
> appropriately readable/writeable pipe which receives a POLLHUP
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH 4/4] tools/libxl: Initialise the fd of the unused half of a datacopier

[Wei Liu]

On Fri, Jul 17, 2015 at 05:51:18PM +0100, Andrew Cooper wrote:
> This bug causes a spurious failure if stdin happens to be an
> appropriately readable/writeable pipe which receives a POLLHUP
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxl/libxl_stream_read.c  |    1 +
>  tools/libxl/libxl_stream_write.c |    1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
> index 3e1cd2a..32a3551 100644
> --- a/tools/libxl/libxl_stream_read.c
> +++ b/tools/libxl/libxl_stream_read.c
> @@ -611,6 +611,7 @@ static void write_emulator_blob(libxl__egc *egc,
>      dc->writewhat  = "qemu save file";
>      dc->copywhat   = "restore v2 stream";
>      dc->writefd    = writefd;
> +    dc->readfd     = -1;
>      dc->maxsz      = -1;
>      dc->callback   = write_emulator_done;
>  
> diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
> index 94f681b..47f71c0 100644
> --- a/tools/libxl/libxl_stream_write.c
> +++ b/tools/libxl/libxl_stream_write.c
> @@ -389,6 +389,7 @@ static void write_emulator_record(libxl__egc *egc,
>      dc->readwhat      = "qemu save file";
>      dc->copywhat      = "save v2 stream";
>      dc->readfd        = readfd;
> +    dc->readfd        = -1;

writefd

>      dc->maxsz         = -1;
>      dc->readbuf       = stream->emu_body + sizeof(*ehdr);
>      dc->bytes_to_read = rec->length - sizeof(*ehdr);
> -- 
> 1.7.10.4

Re: [PATCH 1/4] tools/libxc: Identify the path of the kernel image which cannot be found

[Wei Liu]

On Fri, Jul 17, 2015 at 05:51:15PM +0100, Andrew Cooper wrote:
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

> 
> ---
> This is rather quicker than using strace to find out that I had
> forgotten to ./configure --prefix=/usr
> ---
>  tools/libxc/xg_private.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libxc/xg_private.c b/tools/libxc/xg_private.c
> index c52cb44..507b205 100644
> --- a/tools/libxc/xg_private.c
> +++ b/tools/libxc/xg_private.c
> @@ -37,7 +37,7 @@ char *xc_read_image(xc_interface *xch,
>  
>      if ( (kernel_fd = open(filename, O_RDONLY)) < 0 )
>      {
> -        PERROR("Could not open kernel image");
> +        PERROR("Could not open kernel image '%s'", filename);
>          goto out;
>      }
>  
> -- 
> 1.7.10.4

Re: [PATCH 4/4] tools/libxl: Initialise the fd of the unused half of a datacopier

[Andrew Cooper]

On 17/07/15 17:55, Wei Liu wrote:
> On Fri, Jul 17, 2015 at 05:51:18PM +0100, Andrew Cooper wrote:
>> This bug causes a spurious failure if stdin happens to be an
>> appropriately readable/writeable pipe which receives a POLLHUP
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> ---
>>  tools/libxl/libxl_stream_read.c  |    1 +
>>  tools/libxl/libxl_stream_write.c |    1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
>> index 3e1cd2a..32a3551 100644
>> --- a/tools/libxl/libxl_stream_read.c
>> +++ b/tools/libxl/libxl_stream_read.c
>> @@ -611,6 +611,7 @@ static void write_emulator_blob(libxl__egc *egc,
>>      dc->writewhat  = "qemu save file";
>>      dc->copywhat   = "restore v2 stream";
>>      dc->writefd    = writefd;
>> +    dc->readfd     = -1;
>>      dc->maxsz      = -1;
>>      dc->callback   = write_emulator_done;
>>  
>> diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
>> index 94f681b..47f71c0 100644
>> --- a/tools/libxl/libxl_stream_write.c
>> +++ b/tools/libxl/libxl_stream_write.c
>> @@ -389,6 +389,7 @@ static void write_emulator_record(libxl__egc *egc,
>>      dc->readwhat      = "qemu save file";
>>      dc->copywhat      = "save v2 stream";
>>      dc->readfd        = readfd;
>> +    dc->readfd        = -1;
> writefd

Oops :s yes.

~Andrew

[PATCH v2 4/4] tools/libxl: Initialise the fd of the unused half of a datacopier

[Andrew Cooper]

This bug causes a spurious failure if stdin happens to be an
appropriately readable/writeable pipe which receives a POLLHUP

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>

---
Correct writefd in libxl_stream_write.c
---
 tools/libxl/libxl_stream_read.c  |    1 +
 tools/libxl/libxl_stream_write.c |    1 +
 2 files changed, 2 insertions(+)

diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index 3e1cd2a..32a3551 100644
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -611,6 +611,7 @@ static void write_emulator_blob(libxl__egc *egc,
     dc->writewhat  = "qemu save file";
     dc->copywhat   = "restore v2 stream";
     dc->writefd    = writefd;
+    dc->readfd     = -1;
     dc->maxsz      = -1;
     dc->callback   = write_emulator_done;
 
diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
index 94f681b..5bff52b 100644
--- a/tools/libxl/libxl_stream_write.c
+++ b/tools/libxl/libxl_stream_write.c
@@ -389,6 +389,7 @@ static void write_emulator_record(libxl__egc *egc,
     dc->readwhat      = "qemu save file";
     dc->copywhat      = "save v2 stream";
     dc->readfd        = readfd;
+    dc->writefd       = -1;
     dc->maxsz         = -1;
     dc->readbuf       = stream->emu_body + sizeof(*ehdr);
     dc->bytes_to_read = rec->length - sizeof(*ehdr);
-- 
1.7.10.4

Re: [PATCH v2 4/4] tools/libxl: Initialise the fd of the unused half of a datacopier

[Wei Liu]

On Fri, Jul 17, 2015 at 05:59:09PM +0100, Andrew Cooper wrote:
> This bug causes a spurious failure if stdin happens to be an
> appropriately readable/writeable pipe which receives a POLLHUP
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> 

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
> Correct writefd in libxl_stream_write.c
> ---
>  tools/libxl/libxl_stream_read.c  |    1 +
>  tools/libxl/libxl_stream_write.c |    1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
> index 3e1cd2a..32a3551 100644
> --- a/tools/libxl/libxl_stream_read.c
> +++ b/tools/libxl/libxl_stream_read.c
> @@ -611,6 +611,7 @@ static void write_emulator_blob(libxl__egc *egc,
>      dc->writewhat  = "qemu save file";
>      dc->copywhat   = "restore v2 stream";
>      dc->writefd    = writefd;
> +    dc->readfd     = -1;
>      dc->maxsz      = -1;
>      dc->callback   = write_emulator_done;
>  
> diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
> index 94f681b..5bff52b 100644
> --- a/tools/libxl/libxl_stream_write.c
> +++ b/tools/libxl/libxl_stream_write.c
> @@ -389,6 +389,7 @@ static void write_emulator_record(libxl__egc *egc,
>      dc->readwhat      = "qemu save file";
>      dc->copywhat      = "save v2 stream";
>      dc->readfd        = readfd;
> +    dc->writefd       = -1;
>      dc->maxsz         = -1;
>      dc->readbuf       = stream->emu_body + sizeof(*ehdr);
>      dc->bytes_to_read = rec->length - sizeof(*ehdr);
> -- 
> 1.7.10.4

Re: [PATCH v2 4/4] tools/libxl: Initialise the fd of the unused half of a datacopier

[Ian Campbell]

On Fri, 2015-07-17 at 18:01 +0100, Wei Liu wrote:
> On Fri, Jul 17, 2015 at 05:59:09PM +0100, Andrew Cooper wrote:
> > This bug causes a spurious failure if stdin happens to be an
> > appropriately readable/writeable pipe which receives a POLLHUP
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > CC: Ian Campbell <Ian.Campbell@citrix.com>
> > CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > CC: Wei Liu <wei.liu2@citrix.com>
> > 
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>

I've pushed just this one in the hopes of unblocking the testing over
the weekend (I've got to go soon is the only reason I didn't touch the
other three).

I had to rebase a little to account for the other patches, hopefully I
didn't bugger it up!

Ian.

Re: [PATCH v2 4/4] tools/libxl: Initialise the fd of the unused half of a datacopier

[Andrew Cooper]

On 17/07/15 18:11, Ian Campbell wrote:
> On Fri, 2015-07-17 at 18:01 +0100, Wei Liu wrote:
>> On Fri, Jul 17, 2015 at 05:59:09PM +0100, Andrew Cooper wrote:
>>> This bug causes a spurious failure if stdin happens to be an
>>> appropriately readable/writeable pipe which receives a POLLHUP
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>>> CC: Wei Liu <wei.liu2@citrix.com>
>>>
>> Acked-by: Wei Liu <wei.liu2@citrix.com>
> I've pushed just this one in the hopes of unblocking the testing over
> the weekend (I've got to go soon is the only reason I didn't touch the
> other three).
>
> I had to rebase a little to account for the other patches, hopefully I
> didn't bugger it up!

Ah yes - you would have to.

What is in there looks correct.

~Andrew

Re: [PATCH 2/4] tools/libxl: Log the subject fd in datacopier messages

[Wei Liu]

On Fri, Jul 17, 2015 at 05:51:16PM +0100, Andrew Cooper wrote:
> This is a substantial aid to debugging
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH 3/4] tools/libxl: Identify copywhat in stream v2 datacopiers

[Wei Liu]

On Fri, Jul 17, 2015 at 05:51:17PM +0100, Andrew Cooper wrote:
> This is an aid to debugging
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH 0/4] Fix to libxl migration v2 issue blocking OSSTest

[Wei Liu]

On Fri, Jul 17, 2015 at 05:51:14PM +0100, Andrew Cooper wrote:
> And three improvements to debugging.
> 
> Note that there is still a bug in libxl__toolstack_save() which
> valgrind identified, but I do not wish to block this bugfix on that
> 
> Andrew Cooper (4):
>   tools/libxc: Identify the path of the kernel image which cannot be
>     found
>   tools/libxl: Log the subject fd in datacopier messages
>   tools/libxl: Identify copywhat in stream v2 datacopiers

I think all three patches should wait until next development window
opens unless we have nothing else in our queue (which doesn't seem to be
the case at the moment).

Wei.

>   tools/libxl: Initialise the fd of the unused half of a datacopier
> 
>  tools/libxc/xg_private.c         |    2 +-
>  tools/libxl/libxl_aoutils.c      |   24 ++++++++++++------------
>  tools/libxl/libxl_stream_read.c  |   19 +++++++++++--------
>  tools/libxl/libxl_stream_write.c |    5 ++++-
>  4 files changed, 28 insertions(+), 22 deletions(-)
> 
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 0/4] Fix to libxl migration v2 issue blocking OSSTest

[Andrew Cooper]

On 20/07/15 10:56, Wei Liu wrote:
> On Fri, Jul 17, 2015 at 05:51:14PM +0100, Andrew Cooper wrote:
>> And three improvements to debugging.
>>
>> Note that there is still a bug in libxl__toolstack_save() which
>> valgrind identified, but I do not wish to block this bugfix on that
>>
>> Andrew Cooper (4):
>>   tools/libxc: Identify the path of the kernel image which cannot be
>>     found
>>   tools/libxl: Log the subject fd in datacopier messages
>>   tools/libxl: Identify copywhat in stream v2 datacopiers
> I think all three patches should wait until next development window
> opens unless we have nothing else in our queue (which doesn't seem to be
> the case at the moment).

You mean delay until 4.7? I disagree.  Without these fixes, debugging
issues is substantially harder than they need to be.

They literally are only adding extra information into existing error
messages.

~Andrew

Re: [PATCH 0/4] Fix to libxl migration v2 issue blocking OSSTest

[Wei Liu]

On Mon, Jul 20, 2015 at 11:11:28AM +0100, Andrew Cooper wrote:
> On 20/07/15 10:56, Wei Liu wrote:
> > On Fri, Jul 17, 2015 at 05:51:14PM +0100, Andrew Cooper wrote:
> >> And three improvements to debugging.
> >>
> >> Note that there is still a bug in libxl__toolstack_save() which
> >> valgrind identified, but I do not wish to block this bugfix on that
> >>
> >> Andrew Cooper (4):
> >>   tools/libxc: Identify the path of the kernel image which cannot be
> >>     found
> >>   tools/libxl: Log the subject fd in datacopier messages
> >>   tools/libxl: Identify copywhat in stream v2 datacopiers
> > I think all three patches should wait until next development window
> > opens unless we have nothing else in our queue (which doesn't seem to be
> > the case at the moment).
> 
> You mean delay until 4.7? I disagree.  Without these fixes, debugging
> issues is substantially harder than they need to be.
> 
> They literally are only adding extra information into existing error
> messages.
> 

Well I am expecting two to three big series getting applied soon, any
changes that gets applied now has the chance of forcing those series to
be rebased.

Let's at least wait until next week.

Wei.

> ~Andrew

Re: [PATCH 0/4] Fix to libxl migration v2 issue blocking OSSTest

[Ian Campbell]

On Tue, 2015-07-21 at 13:49 +0100, Wei Liu wrote:
> On Mon, Jul 20, 2015 at 11:11:28AM +0100, Andrew Cooper wrote:
> > On 20/07/15 10:56, Wei Liu wrote:
> > > On Fri, Jul 17, 2015 at 05:51:14PM +0100, Andrew Cooper wrote:
> > > > And three improvements to debugging.
> > > > 
> > > > Note that there is still a bug in libxl__toolstack_save() which
> > > > valgrind identified, but I do not wish to block this bugfix on 
> > > > that
> > > > 
> > > > Andrew Cooper (4):
> > > >   tools/libxc: Identify the path of the kernel image which 
> > > > cannot be
> > > >     found
> > > >   tools/libxl: Log the subject fd in datacopier messages
> > > >   tools/libxl: Identify copywhat in stream v2 datacopiers
> > > I think all three patches should wait until next development 
> > > window
> > > opens unless we have nothing else in our queue (which doesn't 
> > > seem to be
> > > the case at the moment).
> > 
> > You mean delay until 4.7? I disagree.  Without these fixes, 
> > debugging
> > issues is substantially harder than they need to be.
> > 
> > They literally are only adding extra information into existing 
> > error
> > messages.
> > 
> 
> Well I am expecting two to three big series getting applied soon, any
> changes that gets applied now has the chance of forcing those series 
> to
> be rebased.

Wei and I discussed this IRL, the concern was the outstanding colopre
patches.

However I did a test apply on top of 	
https://github.com/macrosheep/xen.git#colo/colo-v9 (the latest colopre)
and there were no rejects due to the remus refactoring.

There were rejects because I already applied 4/4 on Friday, i.e. they
were the inverse of what I fixed up then.

So given the lack of interaction with colopre Wei gave me permission to
go ahead, so I have applied patches 1..3.

4 was applied already, of course.

Ian.

Re: [PATCH 0/4] Fix to libxl migration v2 issue blocking OSSTest

[Ian Campbell]

On Tue, 2015-07-21 at 15:51 +0100, Ian Campbell wrote:
> On Tue, 2015-07-21 at 13:49 +0100, Wei Liu wrote:
> > On Mon, Jul 20, 2015 at 11:11:28AM +0100, Andrew Cooper wrote:
> > > On 20/07/15 10:56, Wei Liu wrote:
> > > > On Fri, Jul 17, 2015 at 05:51:14PM +0100, Andrew Cooper wrote:
> > > > > And three improvements to debugging.
> > > > > 
> > > > > Note that there is still a bug in libxl__toolstack_save() 
> > > > > which
> > > > > valgrind identified, but I do not wish to block this bugfix 
> > > > > on 
> > > > > that
> > > > > 
> > > > > Andrew Cooper (4):
> > > > >   tools/libxc: Identify the path of the kernel image which 
> > > > > cannot be
> > > > >     found
> > > > >   tools/libxl: Log the subject fd in datacopier messages
> > > > >   tools/libxl: Identify copywhat in stream v2 datacopiers
> > > > I think all three patches should wait until next development 
> > > > window
> > > > opens unless we have nothing else in our queue (which doesn't 
> > > > seem to be
> > > > the case at the moment).
> > > 
> > > You mean delay until 4.7? I disagree.  Without these fixes, 
> > > debugging
> > > issues is substantially harder than they need to be.
> > > 
> > > They literally are only adding extra information into existing 
> > > error
> > > messages.
> > > 
> > 
> > Well I am expecting two to three big series getting applied soon, 
> > any
> > changes that gets applied now has the chance of forcing those 
> > series 
> > to
> > be rebased.
> 
> Wei and I discussed this IRL, the concern was the outstanding colopre
> patches.
> 
> However I did a test apply on top of 	
> https://github.com/macrosheep/xen.git#colo/colo-v9 (the latest 
> colopre)
> and there were no rejects due to the remus refactoring.
> 
> There were rejects because I already applied 4/4 on Friday, i.e. they
> were the inverse of what I fixed up then.
> 
> So given the lack of interaction with colopre Wei gave me permission 
> to
> go ahead, so I have applied patches 1..3.
> 
> 4 was applied already, of course.

In doing this I managed to revert part of #4, thanks to Andy for
noticing so promptly.

I've pushed the following:

>From 1287ac109c44ca9b99eb642316d7af83b4081b52 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Tue, 21 Jul 2015 16:00:19 +0100
Subject: [PATCH] tools: libxl: Refix "Initialise the fd of the unused half of
 a datacopier"

Applying the series out of order led to d72befc35f31 "tools/libxl:
Identify copywhat in stream v2 datacopiers" unintentionally reverting
part of 21d9b079e538 "tools/libxl: Initialise the fd of the unused
half of a datacopier".

Put this back.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_stream_read.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index 3e1cd2a..32a3551 100644
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -611,6 +611,7 @@ static void write_emulator_blob(libxl__egc *egc,
     dc->writewhat  = "qemu save file";
     dc->copywhat   = "restore v2 stream";
     dc->writefd    = writefd;
+    dc->readfd     = -1;
     dc->maxsz      = -1;
     dc->callback   = write_emulator_done;
 
-- 
2.1.4