Re: [libvirt] [PATCH] libxl: rework reference counting

2015-06-17 Thread Anthony PERARD
On Tue, Jun 16, 2015 at 10:53:15AM +0100, Anthony PERARD wrote:
> On Mon, Jun 15, 2015 at 08:36:47PM -0600, Jim Fehlig wrote:
> > Similar to commit 540c339a for the QEMU driver, rework reference
> > counting in the libxl driver to make it more deterministic and
> > the code a bit cleaner.
> > 
> > Signed-off-by: Jim Fehlig 
> > ---
> > 
> > I've been testing this patch on and off for a few weeks now using
> > libvirt-tck domain tests, local test scripts, and some manual tests
> > for good measure.  I sent the patch to Anthony Perard (cc'd) nearly
> > two weeks ago for testing in his OpenStack+Xen+libvirt CI loop,
> > although I haven't received any feedback thus far.  Also included
> > Martin in the cc since he encouraged this patch
> 
> But I should be able to test the patch now.

Hi, I gave this patch a try with OpenStack. I have applied the patch on top
of libvirt 1.4.14 + (f86ae40 894d2ff 6dfec1e), which we are using for our CI.

With the patch, libvirt works fine, but I can still have the rare error
"domain 205 device model: startup timed out".

Thanks,

-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] libxl: rework reference counting

2015-06-16 Thread Anthony PERARD
On Mon, Jun 15, 2015 at 08:36:47PM -0600, Jim Fehlig wrote:
> Similar to commit 540c339a for the QEMU driver, rework reference
> counting in the libxl driver to make it more deterministic and
> the code a bit cleaner.
> 
> Signed-off-by: Jim Fehlig 
> ---
> 
> I've been testing this patch on and off for a few weeks now using
> libvirt-tck domain tests, local test scripts, and some manual tests
> for good measure.  I sent the patch to Anthony Perard (cc'd) nearly
> two weeks ago for testing in his OpenStack+Xen+libvirt CI loop,
> although I haven't received any feedback thus far.  Also included
> Martin in the cc since he encouraged this patch

Sorry, I could not really test the patch, since OpenStack master was broken
(deploying OpenStack with devstack). Also I don't want to apply patches
to the Xen Project OpenStack CI loop just to test them.

But I should be able to test the patch now.

-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] libxl: Add timestamp to the libxl driver log.

2015-06-08 Thread Anthony PERARD
Signed-off-by: Anthony PERARD 
---
 src/libxl/libxl_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 9b258ac..e845759 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1541,7 +1541,7 @@ libxlDriverConfigNew(void)
 
 cfg->logger =
 (xentoollog_logger *)xtl_createlogger_stdiostream(cfg->logger_file,
-  XTL_DEBUG, 0);
+  XTL_DEBUG, XTL_STDIOSTREAM_SHOW_DATE);
 if (!cfg->logger) {
 VIR_ERROR(_("cannot create logger for libxenlight, disabling driver"));
     goto error;
-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [Xen-devel] [PATCH 0/3] libxl: domain destroy fixes

2015-03-30 Thread Anthony PERARD
On Wed, Mar 25, 2015 at 02:08:33PM -0600, Jim Fehlig wrote:
> This small series of patches fixes some issues wrt domain destroy in
> the libxl driver.  The primary motivation for this work is to
> prevent locking the virDomainObj during long running destroy operations
> on large memory domains.
> 
> Patch 1 moves job acquisition from libxlDomainStart to it's callers so
> they have more control over when the job is acquired.  Patch 2 fixes a
> few spots where we never acquired a job during domain destroy.  Patch 3
> contains the interesting change, where the virDomainObj is unlocked
> during the long-running destroy operation.
> 
> This series wraps up my work to improve parallel OpenStack Tempest runs
> against the libxl driver.  With libvirt.git master + this series + a
> patched libxl [1], I've successfully run a reproducer that was hitting
> the same issues encountered by Tempest.
> 
> [1] libxl commits from xen.git: 93699882d, f1335f0d, 4783c99a, 1c91d6fba,
> and 188e9c54.  I'll contact the stable branch maintainers and ask them
> to include these commits in the next Xen 4.4.x and 4.5.x releases.

Hi,

I gave a try to this series with OpenStack Tempest. And it is much better.

Thanks!

-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] libvirt/libxl implemetation of get_online_cpu / virNodeGetCPUMap?

2015-02-24 Thread Anthony PERARD
On Tue, Feb 24, 2015 at 01:22:19PM +, Daniel P. Berrange wrote:
> On Tue, Feb 24, 2015 at 01:15:57PM +0000, Anthony PERARD wrote:
> > On Tue, Feb 24, 2015 at 12:46:44PM +, Daniel P. Berrange wrote:
> > > On Tue, Feb 24, 2015 at 12:41:01PM +0000, Anthony PERARD wrote:
> > > > Hi,
> > > > 
> > > > A recent OpenStack nova commit make use of virNodeGetCPUMap to get the 
> > > > list
> > > > of online cpu of a host. But this API is not implemented for the libvirt
> > > > xen driver.
> > > > 
> > > > The commit:
> > > >   Add handling for offlined CPUs to the nova libvirt driver.
> > > > https://review.openstack.org/gitweb?p=openstack/nova.git;a=commitdiff;h=0696a5cd5f0fdc08951a074961bb8ce0c3310086
> > > 
> > > FWIW, this should not impact Xen based on my understanding. The code
> > > path in question should only be used when Nova is setup todo NUMA
> > > pinning support, and that is not supported with Xen in OpenStack,
> > > only KVM.  Did it actually cause failures for you, or are you simply
> > > keeping track of all used APIs in Nova as a sanity check ?
> > 
> > It prevent nova from starting. I do the setup with DevStack.
> > 
> > The error:
> > libvirtError: this function is not supported by the connection driver: 
> > virNodeGetCPUMap
> > 
> > And a part of the traceback:
> >   File "/opt/stack/nova/nova/openstack/common/service.py", line 491, in 
> > run_service
> > service.start()
> >   File "/opt/stack/nova/nova/service.py", line 181, in start
> > self.manager.pre_start_hook()
> >   File "/opt/stack/nova/nova/compute/manager.py", line 1188, in 
> > pre_start_hook
> > self.update_available_resource(nova.context.get_admin_context())
> >   File "/opt/stack/nova/nova/compute/manager.py", line 6062, in 
> > update_available_resource
> > rt.update_available_resource(context)
> >   File "/opt/stack/nova/nova/compute/resource_tracker.py", line 315, in 
> > update_available_resource
> > resources = self.driver.get_available_resource(self.nodename)
> >   File "/opt/stack/nova/nova/virt/libvirt/driver.py", line 4896, in 
> > get_available_resource
> > numa_topology = self._get_host_numa_topology()
> >   File "/opt/stack/nova/nova/virt/libvirt/driver.py", line 4749, in 
> > _get_host_numa_topology
> > online_cpus = self._host.get_online_cpus()
> >   File "/opt/stack/nova/nova/virt/libvirt/host.py", line 599, in 
> > get_online_cpus
> > (cpus, cpu_map, online) = self.get_connection().getCPUMap()
> > 
> > I'll look into why nova is going through NUMA code paths then.
> 
> Oh damn, yes, I understand why now. Please file a bug against Nova for
> this, as we must fix it as a high pripority. It was certainly not my
> intention to break Xen when I approved this change

Here is the bug report: https://bugs.launchpad.net/nova/+bug/1425115

Regards,

-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] libvirt/libxl implemetation of get_online_cpu / virNodeGetCPUMap?

2015-02-24 Thread Anthony PERARD
On Tue, Feb 24, 2015 at 12:46:44PM +, Daniel P. Berrange wrote:
> On Tue, Feb 24, 2015 at 12:41:01PM +0000, Anthony PERARD wrote:
> > Hi,
> > 
> > A recent OpenStack nova commit make use of virNodeGetCPUMap to get the list
> > of online cpu of a host. But this API is not implemented for the libvirt
> > xen driver.
> > 
> > The commit:
> >   Add handling for offlined CPUs to the nova libvirt driver.
> > https://review.openstack.org/gitweb?p=openstack/nova.git;a=commitdiff;h=0696a5cd5f0fdc08951a074961bb8ce0c3310086
> 
> FWIW, this should not impact Xen based on my understanding. The code
> path in question should only be used when Nova is setup todo NUMA
> pinning support, and that is not supported with Xen in OpenStack,
> only KVM.  Did it actually cause failures for you, or are you simply
> keeping track of all used APIs in Nova as a sanity check ?

It prevent nova from starting. I do the setup with DevStack.

The error:
libvirtError: this function is not supported by the connection driver: 
virNodeGetCPUMap

And a part of the traceback:
  File "/opt/stack/nova/nova/openstack/common/service.py", line 491, in 
run_service
service.start()
  File "/opt/stack/nova/nova/service.py", line 181, in start
self.manager.pre_start_hook()
  File "/opt/stack/nova/nova/compute/manager.py", line 1188, in pre_start_hook
self.update_available_resource(nova.context.get_admin_context())
  File "/opt/stack/nova/nova/compute/manager.py", line 6062, in 
update_available_resource
rt.update_available_resource(context)
  File "/opt/stack/nova/nova/compute/resource_tracker.py", line 315, in 
update_available_resource
resources = self.driver.get_available_resource(self.nodename)
  File "/opt/stack/nova/nova/virt/libvirt/driver.py", line 4896, in 
get_available_resource
numa_topology = self._get_host_numa_topology()
  File "/opt/stack/nova/nova/virt/libvirt/driver.py", line 4749, in 
_get_host_numa_topology
online_cpus = self._host.get_online_cpus()
  File "/opt/stack/nova/nova/virt/libvirt/host.py", line 599, in get_online_cpus
(cpus, cpu_map, online) = self.get_connection().getCPUMap()

I'll look into why nova is going through NUMA code paths then.

Thanks for your information,

-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] libvirt/libxl implemetation of get_online_cpu / virNodeGetCPUMap?

2015-02-24 Thread Anthony PERARD
Hi,

A recent OpenStack nova commit make use of virNodeGetCPUMap to get the list
of online cpu of a host. But this API is not implemented for the libvirt
xen driver.

The commit:
  Add handling for offlined CPUs to the nova libvirt driver.
https://review.openstack.org/gitweb?p=openstack/nova.git;a=commitdiff;h=0696a5cd5f0fdc08951a074961bb8ce0c3310086

Is there a need to use this under Xen? (Is it possible to have offline
CPU?).
What libxl API those provide this information, if it exist?

I found libxl_get_online_cpus() but that not enough. They want a
bitmap.

Thanks,

-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [Xen-devel] [PATCH 0/2] libxl: fix handling of fd and timer registrations

2015-02-06 Thread Anthony PERARD
On Mon, Feb 02, 2015 at 05:00:34PM -0700, Jim Fehlig wrote:
> This small series fixes some assertions we occasionally see in the
> libxl driver when running libvirt-TCK.  The assertions were due to
> races between destroying per-domain libxl_ctx and receiving fd and
> timer callbacks associated with them.  The races are masked by
> setting DEBUG loglevel in libvirtd.conf, so often missed by
> automated test setups that want DEBUG loglevel.
> 
> Patch 1 actually fixes the assertions.  Patch2 fixes a stupid mistake.
> See the commit messages for details.
> 
> Jim Fehlig (2):
>   libxl: fix fd and timer event handling
>   libxl: Move setup of child processing code to driver initialization
> 
>  src/libxl/libxl_domain.c | 244 
> +--
>  src/libxl/libxl_driver.c | 212 +++-
>  2 files changed, 212 insertions(+), 244 deletions(-)

Hi Jim,

I gave a try to those two patches with OpenStack. Assuming I haven't make any
mistake, it make things worse.

Environment:
  Ubuntu 14.04
  with Xen package install (xen 4.4)
  libvirt: master (47dd6c4)
  Installed OpenStack via DevStack

Test: ./run_tempest.sh tempest.api.compute

Result:
without the patches, the tests run fine, they all succeed.
with the patches, the tests fail AND libvirt became unresponsible.
  Running `virsh -c xen: list` does not return. (or any virsh command)

I have attach a backtrace, if that can help.

-- 
Anthony PERARD
Thread 30 (Thread 0x7fa9cad9f700 (LWP 5872)):
#0  __lll_lock_wait () at 
../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135
#1  0x7fa9cfc02657 in _L_lock_909 () from 
/lib/x86_64-linux-gnu/libpthread.so.0
#2  0x7fa9cfc02480 in __GI___pthread_mutex_lock (mutex=0x7fa9b8189470) at 
../nptl/pthread_mutex_lock.c:79
#3  0x7fa9cfeda44d in virMutexLock (m=m@entry=0x7fa9b8189470) at 
util/virthread.c:88
#4  0x7fa9cfec172e in virObjectLock (anyobj=anyobj@entry=0x7fa9b8189460) at 
util/virobject.c:323
#5  0x7fa9cfef8001 in virDomainObjListFindByName (doms=0x7fa9b8189460, 
name=0x7fa9a000d5e0 "instance-0020") at conf/domain_conf.c:1114
#6  0x7fa9c118b786 in libxlDomainLookupByName (conn=0x7fa9a0007630, 
name=) at libxl/libxl_driver.c:954
#7  0x7fa9cff71356 in virDomainLookupByName (conn=0x7fa9a0007630, 
name=0x7fa9a000d5e0 "instance-0020") at libvirt-domain.c:427
#8  0x7fa9d16fd492 in remoteDispatchDomainLookupByName (server=, msg=, ret=0x7fa9a0006770, args=0x7fa9af90, 
rerr=0x7fa9cad9ec40, 
client=0x7fa9d36470d0) at remote_dispatch.h:5429
#9  remoteDispatchDomainLookupByNameHelper (server=, 
client=0x7fa9d36470d0, msg=, rerr=0x7fa9cad9ec40, 
args=0x7fa9af90, ret=0x7fa9a0006770)
at remote_dispatch.h:5409
#10 0x7fa9d1717992 in virNetServerProgramDispatchCall (msg=0x7fa9d3645fb0, 
client=0x7fa9d36470d0, server=0x7fa9d362b2f0, prog=0x7fa9d363ee90)
at rpc/virnetserverprogram.c:437
#11 virNetServerProgramDispatch (prog=0x7fa9d363ee90, 
server=server@entry=0x7fa9d362b2f0, client=0x7fa9d36470d0, msg=0x7fa9d3645fb0) 
at rpc/virnetserverprogram.c:307
#12 0x7fa9d17119ed in virNetServerProcessMsg (msg=, 
prog=, client=, srv=0x7fa9d362b2f0) at 
rpc/virnetserver.c:172
#13 virNetServerHandleJob (jobOpaque=, opaque=0x7fa9d362b2f0) at 
rpc/virnetserver.c:193
#14 0x7fa9cfedad25 in virThreadPoolWorker 
(opaque=opaque@entry=0x7fa9d361c5f0) at util/virthreadpool.c:144
#15 0x7fa9cfeda21e in virThreadHelper (data=) at 
util/virthread.c:197
#16 0x7fa9cfc00182 in start_thread (arg=0x7fa9cad9f700) at 
pthread_create.c:312
#17 0x7fa9cf92b00d in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:111

Thread 29 (Thread 0x7fa9ca59e700 (LWP 5873)):
#0  0x7fa9cf91dcbd in poll () at ../sysdeps/unix/syscall-template.S:81
#1  0x7fa9cd452fff in ?? () from /usr/lib/libxenlight-4.4.so
#2  0x7fa9cd45441b in ?? () from /usr/lib/libxenlight-4.4.so
#3  0x7fa9cd438d27 in ?? () from /usr/lib/libxenlight-4.4.so
#4  0x7fa9cd439228 in libxl_domain_create_new () from 
/usr/lib/libxenlight-4.4.so
#5  0x7fa9c11846d8 in libxlDomainStart (driver=driver@entry=0x7fa9b818ff60, 
vm=vm@entry=0x7fa9b80f0770, start_paused=start_paused@entry=false, 
restore_fd=restore_fd@entry=-1) at libxl/libxl_domain.c:1044
#6  0x7fa9c1188134 in libxlDomainCreateWithFlags (dom=0x7fa9b0004440, 
flags=0) at libxl/libxl_driver.c:2587
#7  0x7fa9cff8466d in virDomainCreateWithFlags 
(domain=domain@entry=0x7fa9b0004440, flags=0) at libvirt-domain.c:6898
#8  0x7fa9d16ee4a8 in remoteDispatchDomainCreateWithFlags 
(server=, msg=, ret=0x7fa9b000a560, 
args=0x7fa9b00031d0, rerr=0x7fa9ca59dc40, 
client=) at remote_dispatch.h:3252
#9  remoteDispatchDomainCreateWithFlagsHelper (server=, 
client=, msg=, rerr=0x7fa9ca59dc40, 
args=0x7fa9b00031d0, 
ret=0x7fa9b000a560) at remote_dispatch.h:3229
#10 0x7fa9d1717992 in virNetServerProgram

[libvirt] [PATCH V3] libxl: Set path to console on domain startup.

2015-01-15 Thread Anthony PERARD
The path to the pty of a Xen PV console is set only in
virDomainOpenConsole. But this is done too late. A call to
virDomainGetXMLDesc done before OpenConsole will not have the path to
the pty, but a call after OpenConsole will.

e.g. of the current issue.
Starting a domain with ''
Then:
virDomainGetXMLDesc():
  

  

  
virDomainOpenConsole()
virDomainGetXMLDesc():
  

  
  

  

The patch intend to have the TTY path on the first call of GetXMLDesc.
This is done by setting up the path at domain start up instead of in
OpenConsole.

https://bugzilla.redhat.com/show_bug.cgi?id=1170743

Signed-off-by: Anthony PERARD 

---
Change in V3:
  Using aop_console_how from libxl_domain_create_new()
  Ignore empty string that can return libxl_console_get_tty()

Change in V2:
  Adding bug report link.
  Reword the last part of the patch description.
  Cleanup the code.
  Use VIR_FREE before VIR_STRDUP.
  Remove the code from OpenConsole as it is now a duplicate.

CC: Jim Fehlig 
CC: Ian Campbell 
CC: Ian Jackson 
---
 src/libxl/libxl_domain.c | 47 ---
 src/libxl/libxl_driver.c | 15 ---
 2 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 9185117..804f9b9 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1149,6 +1149,42 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv, 
libxl_domain_config *d_config)
 return ret;
 }
 
+static void
+libxlConsoleCallback(libxl_ctx *ctx, libxl_event* ev, void *for_callback)
+{
+virDomainObjPtr vm = for_callback;
+libxlDomainObjPrivatePtr priv = vm->privateData;
+int i;
+
+virObjectLock(vm);
+for (i = 0; i < vm->def->nconsoles; i++) {
+virDomainChrDefPtr chr = vm->def->consoles[i];
+if (chr && chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) {
+libxl_console_type console_type;
+char *console = NULL;
+int ret;
+
+console_type =
+(chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
+ LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
+ret = libxl_console_get_tty(priv->ctx, ev->domid,
+chr->target.port, console_type,
+&console);
+if (!ret) {
+VIR_FREE(chr->source.data.file.path);
+if (console && console[0] != '\0') {
+ignore_value(VIR_STRDUP(chr->source.data.file.path,
+console));
+}
+}
+VIR_FREE(console);
+}
+}
+virObjectUnlock(vm);
+libxl_event_free(ctx, ev);
+}
+
+
 /*
  * Start a domain through libxenlight.
  *
@@ -1173,6 +1209,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 libxl_domain_restore_params params;
 #endif
 virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
+libxl_asyncprogress_how aop_console_how;
 
 libxl_domain_config_init(&d_config);
 
@@ -1242,17 +1279,21 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 
 /* Unlock virDomainObj while creating the domain */
 virObjectUnlock(vm);
+
+aop_console_how.for_callback = vm;
+aop_console_how.callback = libxlConsoleCallback;
 if (restore_fd < 0) {
 ret = libxl_domain_create_new(priv->ctx, &d_config,
-  &domid, NULL, NULL);
+  &domid, NULL, &aop_console_how);
 } else {
 #ifdef LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS
 params.checkpointed_stream = 0;
 ret = libxl_domain_create_restore(priv->ctx, &d_config, &domid,
-  restore_fd, ¶ms, NULL, NULL);
+  restore_fd, ¶ms, NULL,
+  &aop_console_how);
 #else
 ret = libxl_domain_create_restore(priv->ctx, &d_config, &domid,
-  restore_fd, NULL, NULL);
+  restore_fd, NULL, &aop_console_how);
 #endif
 }
 virObjectLock(vm);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index cad5101..fc0949d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3982,10 +3982,8 @@ libxlDomainOpenConsole(virDomainPtr dom,
 {
 virDomainObjPtr vm = NULL;
 int ret = -1;
-libxl_console_type console_type;
 virDomainChrDefPtr chr = NULL;
 libxlDomainObjPrivatePtr priv;
-char *console = NULL;
 
 virCheckFlags(VIR_DOMAIN_CONSOLE_FORCE, -1);
 
@@ -4027,18 +4025,6 @@ libxlDomainOpenConsole(virDomainPtr dom,
 goto cleanup;
 }
 
-console_type =
-(chr

Re: [libvirt] [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup.

2014-12-16 Thread Anthony PERARD
On Tue, Dec 16, 2014 at 09:30:28AM +, Ian Campbell wrote:
> On Mon, 2014-12-15 at 17:07 +0000, Anthony PERARD wrote:
> > On Thu, Dec 11, 2014 at 04:23:15PM +, Ian Campbell wrote:
> > > On Thu, 2014-12-11 at 16:16 +0000, Anthony PERARD wrote:
> > > > On Tue, Dec 09, 2014 at 03:56:02PM +, Ian Campbell wrote:
> > > > > On Tue, 2014-12-09 at 15:39 +, Anthony PERARD wrote:
> > > > > > The path to the pty of a Xen PV console is set only in
> > > > > > virDomainOpenConsole. But this is done too late. A call to
> > > > > > virDomainGetXMLDesc done before OpenConsole will not have the path 
> > > > > > to
> > > > > > the pty, but a call after OpenConsole will.
> > > > > > 
> > > > > > e.g. of the current issue.
> > > > > > Starting a domain with ''
> > > > > > Then:
> > > > > > virDomainGetXMLDesc():
> > > > > >   
> > > > > > 
> > > > > >   
> > > > > > 
> > > > > >   
> > > > > > virDomainOpenConsole()
> > > > > > virDomainGetXMLDesc():
> > > > > >   
> > > > > > 
> > > > > >   
> > > > > >   
> > > > > > 
> > > > > >   
> > > > > > 
> > > > > > The patch intend to have the TTY path on the first call of 
> > > > > > GetXMLDesc.
> > > > > > This is done by setting up the path at domain start up instead of in
> > > > > > OpenConsole.
> > > > > > 
> > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1170743
> > > > > > 
> > > > > > Signed-off-by: Anthony PERARD 
> > > > > > 
> > > > > > ---
> > > > > > Change in V2:
> > > > > >   Adding bug report link.
> > > > > >   Reword the last part of the patch description.
> > > > > >   Cleanup the code.
> > > > > >   Use VIR_FREE before VIR_STRDUP.
> > > > > >   Remove the code from OpenConsole as it is now a duplicate.
> > > > > > ---
> > > > > >  src/libxl/libxl_domain.c | 20 
> > > > > >  src/libxl/libxl_driver.c | 15 ---
> > > > > >  2 files changed, 20 insertions(+), 15 deletions(-)
> > > > > > 
> > > > > > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> > > > > > index 9c62291..325de79 100644
> > > > > > --- a/src/libxl/libxl_domain.c
> > > > > > +++ b/src/libxl/libxl_domain.c
> > > > > > @@ -1290,6 +1290,26 @@ libxlDomainStart(libxlDriverPrivatePtr 
> > > > > > driver, virDomainObjPtr vm,
> > > > > >  if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> > > > > >  goto cleanup_dom;
> > > > > >  
> > > > > > +if (vm->def->nconsoles) {
> > > > > > +virDomainChrDefPtr chr = vm->def->consoles[0];
> > > > > 
> > > > > Given vm->def->nconsoles should we loop and do them all?
> > > > 
> > > > Maybe. libvirt it self will not be able to access any console that is
> > > > not the first one (virDomainOpenConsole only provide access to console
> > > > 0), but a consumer of libvirt could.
> > > > 
> > > > > Also, and I really should know the answer to this (and sorry for not
> > > > > thinking of it earlier), but:
> > > > > 
> > > > > > +ret = libxl_console_get_tty(priv->ctx, vm->def->id,
> > > > > > +chr->target.port, 
> > > > > > console_type,
> > > > > > +&console);
> > > > > 
> > > > > Might this race against xenconsoled writing the node to xenstore and
> > > > > therefore be prone to failing when starting multiple guests all at 
> > > > > once?
> > > > 
> > > > I've look through out libxl, xenconsoled and libvirt, and I could not
> > > > find any synchronisation point. So I guest it's possible.
> > > > 
> > > > > Is there a hook which is called on virsh 

Re: [libvirt] [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup.

2014-12-15 Thread Anthony PERARD
On Thu, Dec 11, 2014 at 04:23:15PM +, Ian Campbell wrote:
> On Thu, 2014-12-11 at 16:16 +0000, Anthony PERARD wrote:
> > On Tue, Dec 09, 2014 at 03:56:02PM +, Ian Campbell wrote:
> > > On Tue, 2014-12-09 at 15:39 +0000, Anthony PERARD wrote:
> > > > The path to the pty of a Xen PV console is set only in
> > > > virDomainOpenConsole. But this is done too late. A call to
> > > > virDomainGetXMLDesc done before OpenConsole will not have the path to
> > > > the pty, but a call after OpenConsole will.
> > > > 
> > > > e.g. of the current issue.
> > > > Starting a domain with ''
> > > > Then:
> > > > virDomainGetXMLDesc():
> > > >   
> > > > 
> > > >   
> > > > 
> > > >   
> > > > virDomainOpenConsole()
> > > > virDomainGetXMLDesc():
> > > >   
> > > > 
> > > >   
> > > >   
> > > > 
> > > >   
> > > > 
> > > > The patch intend to have the TTY path on the first call of GetXMLDesc.
> > > > This is done by setting up the path at domain start up instead of in
> > > > OpenConsole.
> > > > 
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1170743
> > > > 
> > > > Signed-off-by: Anthony PERARD 
> > > > 
> > > > ---
> > > > Change in V2:
> > > >   Adding bug report link.
> > > >   Reword the last part of the patch description.
> > > >   Cleanup the code.
> > > >   Use VIR_FREE before VIR_STRDUP.
> > > >   Remove the code from OpenConsole as it is now a duplicate.
> > > > ---
> > > >  src/libxl/libxl_domain.c | 20 
> > > >  src/libxl/libxl_driver.c | 15 ---
> > > >  2 files changed, 20 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> > > > index 9c62291..325de79 100644
> > > > --- a/src/libxl/libxl_domain.c
> > > > +++ b/src/libxl/libxl_domain.c
> > > > @@ -1290,6 +1290,26 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
> > > > virDomainObjPtr vm,
> > > >  if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> > > >  goto cleanup_dom;
> > > >  
> > > > +if (vm->def->nconsoles) {
> > > > +virDomainChrDefPtr chr = vm->def->consoles[0];
> > > 
> > > Given vm->def->nconsoles should we loop and do them all?
> > 
> > Maybe. libvirt it self will not be able to access any console that is
> > not the first one (virDomainOpenConsole only provide access to console
> > 0), but a consumer of libvirt could.
> > 
> > > Also, and I really should know the answer to this (and sorry for not
> > > thinking of it earlier), but:
> > > 
> > > > +ret = libxl_console_get_tty(priv->ctx, vm->def->id,
> > > > +chr->target.port, console_type,
> > > > +&console);
> > > 
> > > Might this race against xenconsoled writing the node to xenstore and
> > > therefore be prone to failing when starting multiple guests all at once?
> > 
> > I've look through out libxl, xenconsoled and libvirt, and I could not
> > find any synchronisation point. So I guest it's possible.
> > 
> > > Is there a hook which is called on virsh dumpxml which could update
> > > things on the fly (i.e. lookup the console iff it isn't already set)?
> > 
> > I guest we could modify libxlDomainGetXMLDesc and libxlDomainOpenConsole
> > to do the check, or having a xenstore watch on the path (if that is
> > possible).
> 
> The aop_console_how option to libxl_domain_create_new and
> libxl_domain_create_restore is documented as:
> 
>   /* A progress report will be made via ao_console_how, of type
>* domain_create_console_available, when the domain's primary
>* console is available and can be connected to.
>*/
> 
> Which sounds like exactly what is needed?

It looks like the progress is reported before the main function finish,
from the description of the type libxl_asyncprogress_how (in libxl.h).

Also, I tryied to use it, it works if xenconsoled is running. But if
xenconsoled is not running, then the callback is also called and
libxl_console_get_tty() return an empty string.

Or, maybe my test case is not relevant, so another question: Will
libxl wait for xenconsoled to process the new domain before calling the
callback?
Or, should I set the callback to NULL and have the
domain_create_console_available event sent through
the callback set by libxl_event_register_callbacks()?

-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup.

2014-12-11 Thread Anthony PERARD
On Thu, Dec 11, 2014 at 04:23:15PM +, Ian Campbell wrote:
> On Thu, 2014-12-11 at 16:16 +0000, Anthony PERARD wrote:
> > On Tue, Dec 09, 2014 at 03:56:02PM +, Ian Campbell wrote:
> > > On Tue, 2014-12-09 at 15:39 +0000, Anthony PERARD wrote:
> > > > The path to the pty of a Xen PV console is set only in
> > > > virDomainOpenConsole. But this is done too late. A call to
> > > > virDomainGetXMLDesc done before OpenConsole will not have the path to
> > > > the pty, but a call after OpenConsole will.
> > > > 
> > > > e.g. of the current issue.
> > > > Starting a domain with ''
> > > > Then:
> > > > virDomainGetXMLDesc():
> > > >   
> > > > 
> > > >   
> > > > 
> > > >   
> > > > virDomainOpenConsole()
> > > > virDomainGetXMLDesc():
> > > >   
> > > > 
> > > >   
> > > >   
> > > > 
> > > >   
> > > > 
> > > > The patch intend to have the TTY path on the first call of GetXMLDesc.
> > > > This is done by setting up the path at domain start up instead of in
> > > > OpenConsole.
> > > > 
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1170743
> > > > 
> > > > Signed-off-by: Anthony PERARD 
> > > > 
> > > > ---
> > > > Change in V2:
> > > >   Adding bug report link.
> > > >   Reword the last part of the patch description.
> > > >   Cleanup the code.
> > > >   Use VIR_FREE before VIR_STRDUP.
> > > >   Remove the code from OpenConsole as it is now a duplicate.
> > > > ---
> > > >  src/libxl/libxl_domain.c | 20 
> > > >  src/libxl/libxl_driver.c | 15 ---
> > > >  2 files changed, 20 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> > > > index 9c62291..325de79 100644
> > > > --- a/src/libxl/libxl_domain.c
> > > > +++ b/src/libxl/libxl_domain.c
> > > > @@ -1290,6 +1290,26 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
> > > > virDomainObjPtr vm,
> > > >  if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> > > >  goto cleanup_dom;
> > > >  
> > > > +if (vm->def->nconsoles) {
> > > > +virDomainChrDefPtr chr = vm->def->consoles[0];
> > > 
> > > Given vm->def->nconsoles should we loop and do them all?
> > 
> > Maybe. libvirt it self will not be able to access any console that is
> > not the first one (virDomainOpenConsole only provide access to console
> > 0), but a consumer of libvirt could.
> > 
> > > Also, and I really should know the answer to this (and sorry for not
> > > thinking of it earlier), but:
> > > 
> > > > +ret = libxl_console_get_tty(priv->ctx, vm->def->id,
> > > > +chr->target.port, console_type,
> > > > +&console);
> > > 
> > > Might this race against xenconsoled writing the node to xenstore and
> > > therefore be prone to failing when starting multiple guests all at once?
> > 
> > I've look through out libxl, xenconsoled and libvirt, and I could not
> > find any synchronisation point. So I guest it's possible.
> > 
> > > Is there a hook which is called on virsh dumpxml which could update
> > > things on the fly (i.e. lookup the console iff it isn't already set)?
> > 
> > I guest we could modify libxlDomainGetXMLDesc and libxlDomainOpenConsole
> > to do the check, or having a xenstore watch on the path (if that is
> > possible).
> 
> The aop_console_how option to libxl_domain_create_new and
> libxl_domain_create_restore is documented as:
> 
>   /* A progress report will be made via ao_console_how, of type
>* domain_create_console_available, when the domain's primary
>* console is available and can be connected to.
>*/
> 
> Which sounds like exactly what is needed?

It look like it. If I find how to use that.

-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup.

2014-12-11 Thread Anthony PERARD
On Tue, Dec 09, 2014 at 03:56:02PM +, Ian Campbell wrote:
> On Tue, 2014-12-09 at 15:39 +0000, Anthony PERARD wrote:
> > The path to the pty of a Xen PV console is set only in
> > virDomainOpenConsole. But this is done too late. A call to
> > virDomainGetXMLDesc done before OpenConsole will not have the path to
> > the pty, but a call after OpenConsole will.
> > 
> > e.g. of the current issue.
> > Starting a domain with ''
> > Then:
> > virDomainGetXMLDesc():
> >   
> > 
> >   
> > 
> >   
> > virDomainOpenConsole()
> > virDomainGetXMLDesc():
> >   
> > 
> >   
> >   
> > 
> >   
> > 
> > The patch intend to have the TTY path on the first call of GetXMLDesc.
> > This is done by setting up the path at domain start up instead of in
> > OpenConsole.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1170743
> > 
> > Signed-off-by: Anthony PERARD 
> > 
> > ---
> > Change in V2:
> >   Adding bug report link.
> >   Reword the last part of the patch description.
> >   Cleanup the code.
> >   Use VIR_FREE before VIR_STRDUP.
> >   Remove the code from OpenConsole as it is now a duplicate.
> > ---
> >  src/libxl/libxl_domain.c | 20 
> >  src/libxl/libxl_driver.c | 15 ---
> >  2 files changed, 20 insertions(+), 15 deletions(-)
> > 
> > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> > index 9c62291..325de79 100644
> > --- a/src/libxl/libxl_domain.c
> > +++ b/src/libxl/libxl_domain.c
> > @@ -1290,6 +1290,26 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
> > virDomainObjPtr vm,
> >  if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> >  goto cleanup_dom;
> >  
> > +if (vm->def->nconsoles) {
> > +virDomainChrDefPtr chr = vm->def->consoles[0];
> 
> Given vm->def->nconsoles should we loop and do them all?

Maybe. libvirt it self will not be able to access any console that is
not the first one (virDomainOpenConsole only provide access to console
0), but a consumer of libvirt could.

> Also, and I really should know the answer to this (and sorry for not
> thinking of it earlier), but:
> 
> > +ret = libxl_console_get_tty(priv->ctx, vm->def->id,
> > +chr->target.port, console_type,
> > +&console);
> 
> Might this race against xenconsoled writing the node to xenstore and
> therefore be prone to failing when starting multiple guests all at once?

I've look through out libxl, xenconsoled and libvirt, and I could not
find any synchronisation point. So I guest it's possible.

> Is there a hook which is called on virsh dumpxml which could update
> things on the fly (i.e. lookup the console iff it isn't already set)?

I guest we could modify libxlDomainGetXMLDesc and libxlDomainOpenConsole
to do the check, or having a xenstore watch on the path (if that is
possible).

-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH V2] libxl: Set path to console on domain startup.

2014-12-09 Thread Anthony PERARD
The path to the pty of a Xen PV console is set only in
virDomainOpenConsole. But this is done too late. A call to
virDomainGetXMLDesc done before OpenConsole will not have the path to
the pty, but a call after OpenConsole will.

e.g. of the current issue.
Starting a domain with ''
Then:
virDomainGetXMLDesc():
  

  

  
virDomainOpenConsole()
virDomainGetXMLDesc():
  

  
  

  

The patch intend to have the TTY path on the first call of GetXMLDesc.
This is done by setting up the path at domain start up instead of in
OpenConsole.

https://bugzilla.redhat.com/show_bug.cgi?id=1170743

Signed-off-by: Anthony PERARD 

---
Change in V2:
  Adding bug report link.
  Reword the last part of the patch description.
  Cleanup the code.
  Use VIR_FREE before VIR_STRDUP.
  Remove the code from OpenConsole as it is now a duplicate.
---
 src/libxl/libxl_domain.c | 20 
 src/libxl/libxl_driver.c | 15 ---
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 9c62291..325de79 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1290,6 +1290,26 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
 goto cleanup_dom;
 
+if (vm->def->nconsoles) {
+virDomainChrDefPtr chr = vm->def->consoles[0];
+if (chr && chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) {
+libxl_console_type console_type;
+char *console = NULL;
+
+console_type =
+(chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
+ LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
+ret = libxl_console_get_tty(priv->ctx, vm->def->id,
+chr->target.port, console_type,
+&console);
+if (!ret) {
+VIR_FREE(chr->source.data.file.path);
+ignore_value(VIR_STRDUP(chr->source.data.file.path, console));
+}
+VIR_FREE(console);
+}
+}
+
 if (!start_paused) {
 libxl_domain_unpause(priv->ctx, domid);
 virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, 
VIR_DOMAIN_RUNNING_BOOTED);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 53c87ce..e79afac 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3957,10 +3957,8 @@ libxlDomainOpenConsole(virDomainPtr dom,
 {
 virDomainObjPtr vm = NULL;
 int ret = -1;
-libxl_console_type console_type;
 virDomainChrDefPtr chr = NULL;
 libxlDomainObjPrivatePtr priv;
-char *console = NULL;
 
 virCheckFlags(VIR_DOMAIN_CONSOLE_FORCE, -1);
 
@@ -4002,18 +4000,6 @@ libxlDomainOpenConsole(virDomainPtr dom,
 goto cleanup;
 }
 
-console_type =
-(chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
-LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
-
-ret = libxl_console_get_tty(priv->ctx, vm->def->id, chr->target.port,
-console_type, &console);
-if (ret)
-goto cleanup;
-
-if (VIR_STRDUP(chr->source.data.file.path, console) < 0)
-goto cleanup;
-
 /* handle mutually exclusive access to console devices */
 ret = virChrdevOpen(priv->devs,
 &chr->source,
@@ -4027,7 +4013,6 @@ libxlDomainOpenConsole(virDomainPtr dom,
 }
 
  cleanup:
-    VIR_FREE(console);
 if (vm)
 virObjectUnlock(vm);
 return ret;
-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] libxl: Set path to console on domain startup.

2014-12-09 Thread Anthony PERARD
On Mon, Dec 08, 2014 at 12:03:36PM -0700, Jim Fehlig wrote:
> Anthony PERARD wrote:
> > The path to the pty of a Xen PV console is set only in
> > virDomainOpenConsole. But this is done too late. A call to
> > virDomainGetXMLDesc done before OpenConsole will not have the path to
> > the pty, but a call after OpenConsole will.
> >   
> 
> Hi Anthony,
> 
> Thanks for the patch. Can you address the comments made by others, my
> comments below, and provide a V2?

Will do.

> > e.g. of the current issue.
> > Starting a domain with ''
> > Then:
> > virDomainGetXMLDesc():
> >   
> > 
> >   
> > 
> >   
> > virDomainOpenConsole()
> > virDomainGetXMLDesc():
> >   
> > 
> >   
> >   
> > 
> >   
> >
> > The patch intend to get the tty path on the first call of GetXMLDesc.
> >
> > Signed-off-by: Anthony PERARD 
> > ---
> >  src/libxl/libxl_domain.c | 17 +
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> > index 9c62291..de56054 100644
> > --- a/src/libxl/libxl_domain.c
> > +++ b/src/libxl/libxl_domain.c
> > @@ -1290,6 +1290,23 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
> > virDomainObjPtr vm,
> >  if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> >  goto cleanup_dom;
> >  
> > +if (vm->def->nconsoles) {
> > +virDomainChrDefPtr chr = NULL;
> > +chr = vm->def->consoles[0];
> > +if (chr && chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) {
> > +libxl_console_type console_type;
> > +char *console = NULL;
> > +console_type =
> > +(chr->targetType == 
> > VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
> > + LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
> > +ret = libxl_console_get_tty(priv->ctx, vm->def->id, 
> > chr->target.port,
> > +console_type, &console);
> > +if (!ret)
> > +    ignore_value(VIR_STRDUP(chr->source.data.file.path, 
> > console));
> >   
> 
> VIR_STRDUP will not free an existing value. You should VIR_FREE first,
> which btw can handle a NULL argument. And since you're initializing
> source.data.file.path when starting the domain, I think we can drop the
> similar code in libxlDomainOpenConsole().

I will do that.
Thanks,

-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] libxl: Set path to console on domain startup

2014-12-08 Thread Anthony PERARD
On Mon, Dec 08, 2014 at 04:40:04PM +0100, Ján Tomko wrote:
> On 12/05/2014 05:30 PM, Anthony PERARD wrote:
> > Hi,
> > 
> > I'm trying to fix an issue when using OpenStack with libvirt+xen 
> > (libxenlight).
> > OpenStack cannot access the console output of a Xen PV guest, because the 
> > XML
> > generated by libvirt for a domain is missing the path to the pty. The path
> > actually appear in the XML once one call virDomainOpenConsole().
> > 
> > The patch intend to get the path to the pty without having to call
> > virDomainOpenConsole, so I've done the work in libxlDomainStart(). So I 
> > have a
> > few question:
> > Is libxlDomainStart will be called on restore/migrate/reboot?
> > I guest the function libxlDomainOpenConsole() would not need to do the same
> > work if the console path is settup properly.
> > 
> > There is a bug report about this:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1170743
> > 
> 
> Hi,
> 
> you can leave the bugzilla link in the commit message, if somebody ever needs
> more context.

Ok.

> (And the patch looks good to me, but I'm no libxl expert)

Thanks,

-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [Xen-devel] [PATCH] libxl: Set path to console on domain startup.

2014-12-08 Thread Anthony PERARD
On Mon, Dec 08, 2014 at 11:59:44AM +, Ian Campbell wrote:
> On Fri, 2014-12-05 at 16:30 +0000, Anthony PERARD wrote:
> 
> Jim Fehlig maintains the libxl driver in libvirt, so you should CC him
> (I've done so here...)

Thanks.

> > The path to the pty of a Xen PV console is set only in
> > virDomainOpenConsole. But this is done too late. A call to
> > virDomainGetXMLDesc done before OpenConsole will not have the path to
> > the pty, but a call after OpenConsole will.
> > 
> > e.g. of the current issue.
> > Starting a domain with ''
> > Then:
> > virDomainGetXMLDesc():
> >   
> > 
> >   
> > 
> >   
> > virDomainOpenConsole()
> > virDomainGetXMLDesc():
> >   
> > 
> >   
> >   
> > 
> >   
> > 
> > The patch intend to get the tty path on the first call of GetXMLDesc.
> 
> Doesn't it actually do it on domain start (which makes more sense to me
> anyway).

Just a wording issue. I meant: Have GetXMLDesc always return the path to
the tty.

> > 
> > Signed-off-by: Anthony PERARD 
> > ---
> >  src/libxl/libxl_domain.c | 17 +
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> > index 9c62291..de56054 100644
> > --- a/src/libxl/libxl_domain.c
> > +++ b/src/libxl/libxl_domain.c
> > @@ -1290,6 +1290,23 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
> > virDomainObjPtr vm,
> >  if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> >  goto cleanup_dom;
> >  
> > +if (vm->def->nconsoles) {
> > +virDomainChrDefPtr chr = NULL;
> > +chr = vm->def->consoles[0];
> > +if (chr && chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) {
> > +libxl_console_type console_type;
> > +char *console = NULL;
> > +console_type =
> > +(chr->targetType == 
> > VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
> > + LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
> > +ret = libxl_console_get_tty(priv->ctx, vm->def->id, 
> > chr->target.port,
> > +console_type, &console);
> > +if (!ret)
> > +ignore_value(VIR_STRDUP(chr->source.data.file.path, 
> > console));
> 
> libxlDomainOpenConsole will strdup another (well, probably the same)
> value here, causing a leak I think, so you'll need some check there too
> I expect.

So, if OpenConsole is call twice, there will also be a leak? Or maybe it
can not be called twice.

Anyway, I though from the use of VIR_STRDUP there that it was safe to
call VIR_STRDUP several times and it well free the destination. I might
be wrong.

> > +VIR_FREE(console);
> > +}
> > +}
> > +
> >  if (!start_paused) {
> >  libxl_domain_unpause(priv->ctx, domid);
> >  virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, 
> > VIR_DOMAIN_RUNNING_BOOTED);

-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] libxl: Set path to console on domain startup

2014-12-05 Thread Anthony PERARD
Hi,

I'm trying to fix an issue when using OpenStack with libvirt+xen (libxenlight).
OpenStack cannot access the console output of a Xen PV guest, because the XML
generated by libvirt for a domain is missing the path to the pty. The path
actually appear in the XML once one call virDomainOpenConsole().

The patch intend to get the path to the pty without having to call
virDomainOpenConsole, so I've done the work in libxlDomainStart(). So I have a
few question:
Is libxlDomainStart will be called on restore/migrate/reboot?
I guest the function libxlDomainOpenConsole() would not need to do the same
work if the console path is settup properly.

There is a bug report about this:
https://bugzilla.redhat.com/show_bug.cgi?id=1170743

Regards,

Anthony PERARD (1):
  libxl: Set path to console on domain startup.

 src/libxl/libxl_domain.c | 17 +
 1 file changed, 17 insertions(+)

-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] libxl: Set path to console on domain startup.

2014-12-05 Thread Anthony PERARD
The path to the pty of a Xen PV console is set only in
virDomainOpenConsole. But this is done too late. A call to
virDomainGetXMLDesc done before OpenConsole will not have the path to
the pty, but a call after OpenConsole will.

e.g. of the current issue.
Starting a domain with ''
Then:
virDomainGetXMLDesc():
  

  

  
virDomainOpenConsole()
virDomainGetXMLDesc():
  

  
  

  

The patch intend to get the tty path on the first call of GetXMLDesc.

Signed-off-by: Anthony PERARD 
---
 src/libxl/libxl_domain.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 9c62291..de56054 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1290,6 +1290,23 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
 goto cleanup_dom;
 
+if (vm->def->nconsoles) {
+virDomainChrDefPtr chr = NULL;
+chr = vm->def->consoles[0];
+if (chr && chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) {
+libxl_console_type console_type;
+char *console = NULL;
+console_type =
+(chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
+ LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
+ret = libxl_console_get_tty(priv->ctx, vm->def->id, 
chr->target.port,
+console_type, &console);
+if (!ret)
+ignore_value(VIR_STRDUP(chr->source.data.file.path, console));
+VIR_FREE(console);
+}
+}
+
 if (!start_paused) {
 libxl_domain_unpause(priv->ctx, domid);
 virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, 
VIR_DOMAIN_RUNNING_BOOTED);
-- 
Anthony PERARD

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list