Re: [PATCH] libxl: fix xenstore connection when run in domU

Re: [PATCH] libxl: fix xenstore connection when run in domU

[Jun Zhu (Intern)]

Hi

There are other places that use the xs_daemon_open in libxl. Is it necessory to change them as follows?
     ctx->xsh = xs_daemon_open();
+    if (!ctx->xsh)
+        ctx->xsh = xs_domain_open();
     if (!ctx->xsh) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno, 
                         "cannot connect to xenstore");

Jun Zhu
Citrix Systems UK

Re: Re: [PATCH] libxl: fix xenstore connection when run in domU

[Ian Campbell]

On Tue, 2010-08-31 at 10:05 +0100, Jun Zhu (Intern) wrote:
> Hi
> 
> There are other places that use the xs_daemon_open in libxl. Is it necessory to change them as follows?
>      ctx->xsh = xs_daemon_open();
> +    if (!ctx->xsh)
> +        ctx->xsh = xs_domain_open();
>      if (!ctx->xsh) {
>          XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno, 
>                          "cannot connect to xenstore");

I think we'd be better off introducing "libxl__xs_open" which does the
right thing and make the users to all use it.

Is there any harm in always preferring xs_domain_open, even if the
xenstore daemon happens to be co-located in the same domain?

Ian.

RE: Re: [PATCH] libxl: fix xenstore connection when run in domU

[Jun Zhu (Intern)]

Hi,

I don't think it is a good idea to let users decide which "open". If we check if it is in Dom0, it uses xs_daemon_open; if in DomU, it uses xs_domain_open. Is it better?

If it is always preferring xs_domain_open, the performance is not better than that of xs_daemon_open when running in Dom0.

Jun Zhu
Citrix Systems UK
________________________________________
From: Ian Campbell
Sent: Tuesday, August 31, 2010 5:15 AM
To: Jun Zhu (Intern)
Cc: xen-devel@lists.xensource.com; dgdegra@tycho.nsa.gov
Subject: Re: [Xen-devel] Re: [PATCH] libxl: fix xenstore connection when run in domU

On Tue, 2010-08-31 at 10:05 +0100, Jun Zhu (Intern) wrote:
> Hi
>
> There are other places that use the xs_daemon_open in libxl. Is it necessory to change them as follows?
>      ctx->xsh = xs_daemon_open();
> +    if (!ctx->xsh)
> +        ctx->xsh = xs_domain_open();
>      if (!ctx->xsh) {
>          XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno,
>                          "cannot connect to xenstore");

I think we'd be better off introducing "libxl__xs_open" which does the
right thing and make the users to all use it.

Is there any harm in always preferring xs_domain_open, even if the
xenstore daemon happens to be co-located in the same domain?

Ian.

RE: Re: [PATCH] libxl: fix xenstore connection when run in domU

[Ian Campbell]

On Tue, 2010-08-31 at 10:36 +0100, Jun Zhu (Intern) wrote:
> I don't think it is a good idea to let users decide which "open". If
> we check if it is in Dom0, it uses xs_daemon_open; if in DomU, it uses
> xs_domain_open. Is it better?

I meant all the users internal to libxl, not the users of the library,
(note that the libxl__ namespace is internal to the library). This
choice should indeed be completely hidden from libxl's users.

Ian.

Re: Re: [PATCH] libxl: fix xenstore connection when run in domU

[Tim Deegan]

At 10:15 +0100 on 31 Aug (1283249719), Ian Campbell wrote:
> On Tue, 2010-08-31 at 10:05 +0100, Jun Zhu (Intern) wrote:
> > Hi
> > 
> > There are other places that use the xs_daemon_open in libxl. Is it necessory to change them as follows?
> >      ctx->xsh = xs_daemon_open();
> > +    if (!ctx->xsh)
> > +        ctx->xsh = xs_domain_open();
> >      if (!ctx->xsh) {
> >          XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno, 
> >                          "cannot connect to xenstore");
> 
> I think we'd be better off introducing "libxl__xs_open" which does the
> right thing and make the users to all use it.
> 
> Is there any harm in always preferring xs_domain_open, even if the
> xenstore daemon happens to be co-located in the same domain?

I don't think so; even the performance probably won't be that much
different.  It certainly used to work when we first put Xenstore in its
own domain and anything that's broken since then is a regression worth
fixing.

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

RE: Re: [PATCH] libxl: fix xenstore connection when run in domU

[Jun Zhu (Intern)]

Yes. 
If xenstore runs in a seperate domain, it must use xenbus dev to communicate. If so, the xenstore commands in Xenbus driver are not complete. For example, it does not contain XS_INTRODUCE now. 

Jun Zhu
Citrix Systems UK
________________________________________
From: Tim Deegan
Sent: Tuesday, August 31, 2010 6:28 AM
To: Ian Campbell
Cc: Jun Zhu (Intern); dgdegra@tycho.nsa.gov; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] Re: [PATCH] libxl: fix xenstore connection when        run in domU

At 10:15 +0100 on 31 Aug (1283249719), Ian Campbell wrote:
> On Tue, 2010-08-31 at 10:05 +0100, Jun Zhu (Intern) wrote:
> > Hi
> >
> > There are other places that use the xs_daemon_open in libxl. Is it necessory to change them as follows?
> >      ctx->xsh = xs_daemon_open();
> > +    if (!ctx->xsh)
> > +        ctx->xsh = xs_domain_open();
> >      if (!ctx->xsh) {
> >          XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno,
> >                          "cannot connect to xenstore");
>
> I think we'd be better off introducing "libxl__xs_open" which does the
> right thing and make the users to all use it.
>
> Is there any harm in always preferring xs_domain_open, even if the
> xenstore daemon happens to be co-located in the same domain?

I don't think so; even the performance probably won't be that much
different.  It certainly used to work when we first put Xenstore in its
own domain and anything that's broken since then is a regression worth
fixing.

Tim.

--
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

RE: Re: [PATCH] libxl: fix xenstore connection when run in domU

[Ian Campbell]

On Tue, 2010-08-31 at 11:32 +0100, Jun Zhu (Intern) wrote:
> 
> If xenstore runs in a seperate domain, it must use xenbus dev to
> communicate. If so, the xenstore commands in Xenbus driver are not
> complete. For example, it does not contain XS_INTRODUCE now. 

IIRC Diego's original xenstore-stubdom patch series from way back when
(posted to the list by Diego and again later by Alex Zeffertt) included
patches to the kernel side xenbus driver to resolve issues like this.

Ian.

Re: Re: [PATCH] libxl: fix xenstore connection when run in domU

[Patrick Colp]

Which XenBus driver is this? In Linux? I've certainly had no issues
running a Mini-OS XenStore stubdom (at least not in regards to missing
things like XS_INTRODUCE).


Patrick


On 31 August 2010 03:44, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> On Tue, 2010-08-31 at 11:32 +0100, Jun Zhu (Intern) wrote:
>>
>> If xenstore runs in a seperate domain, it must use xenbus dev to
>> communicate. If so, the xenstore commands in Xenbus driver are not
>> complete. For example, it does not contain XS_INTRODUCE now.
>
> IIRC Diego's original xenstore-stubdom patch series from way back when
> (posted to the list by Diego and again later by Alex Zeffertt) included
> patches to the kernel side xenbus driver to resolve issues like this.
>
> Ian.
>
>
>

Re: Re: [PATCH] libxl: fix xenstore connection when run in domU

[Ian Campbell]

On Tue, 2010-08-31 at 18:51 +0100, Patrick Colp wrote:
> Which XenBus driver is this? In Linux?

Yes. Specifically pvops Linux domain 0.

> I've certainly had no issues running a Mini-OS XenStore stubdom (at least not in regards to missing
> things like XS_INTRODUCE).

xenbus_file_write has an explicit whitelist of XS_FOO which are allowed,
which doesn't include XS_INTRODUCE.

This seems to be different in linux-2.6.18-xen due to 874:68d582b0ad05
and 876:bd7e30b58d12 which I had forgotten were already applied. I guess
those just need porting up to pvops.

Ian.

> 
> 
> Patrick
> 
> 
> On 31 August 2010 03:44, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> > On Tue, 2010-08-31 at 11:32 +0100, Jun Zhu (Intern) wrote:
> >>
> >> If xenstore runs in a seperate domain, it must use xenbus dev to
> >> communicate. If so, the xenstore commands in Xenbus driver are not
> >> complete. For example, it does not contain XS_INTRODUCE now.
> >
> > IIRC Diego's original xenstore-stubdom patch series from way back when
> > (posted to the list by Diego and again later by Alex Zeffertt) included
> > patches to the kernel side xenbus driver to resolve issues like this.
> >
> > Ian.
> >
> >
> >

答复: [Xen-devel] Re: [PATCH] libxl: fix xenstore connection when run in domU

[Jun Zhu (Intern)]

[-- Attachment #1: Type: text/plain, Size: 3959 bytes --]

In pvops, in the function  xenbus_file_write of file drivers/xen/xenfs/xenbus.c.
In the switch, there is no XS_INTRODUCE which is used to create a new domain.
These commands are eliminated here, since in default, a Domu cannot use these commands. 

static ssize_t xenbus_file_write(struct file *filp,
				const char __user *ubuf,
				size_t len, loff_t *ppos)
{
	struct xenbus_file_priv *u = filp->private_data;
	uint32_t msg_type;
	int rc = len;
	int ret;
	LIST_HEAD(staging_q);

	/*
	 * We're expecting usermode to be writing properly formed
	 * xenbus messages.  If they write an incomplete message we
	 * buffer it up.  Once it is complete, we act on it.
	 */

	/*
	 * Make sure concurrent writers can't stomp all over each
	 * other's messages and make a mess of our partial message
	 * buffer.  We don't make any attemppt to stop multiple
	 * writers from making a mess of each other's incomplete
	 * messages; we're just trying to guarantee our own internal
	 * consistency and make sure that single writes are handled
	 * atomically.
	 */
	mutex_lock(&u->msgbuffer_mutex);

	/* Get this out of the way early to avoid confusion */
	if (len == 0)
		goto out;

	/* Can't write a xenbus message larger we can buffer */
	if ((len + u->len) > sizeof(u->u.buffer)) {
		/* On error, dump existing buffer */
		u->len = 0;
		rc = -EINVAL;
		goto out;
	}

	ret = copy_from_user(u->u.buffer + u->len, ubuf, len);

	if (ret == len) {
		rc = -EFAULT;
		goto out;
	}

	/* Deal with a partial copy. */
	len -= ret;
	rc = len;

	u->len += len;

	/* Return if we haven't got a full message yet */
	if (u->len < sizeof(u->u.msg))
		goto out;	/* not even the header yet */

	/* If we're expecting a message that's larger than we can
	   possibly send, dump what we have and return an error. */
	if ((sizeof(u->u.msg) + u->u.msg.len) > sizeof(u->u.buffer)) {
		rc = -E2BIG;
		u->len = 0;
		goto out;
	}

	if (u->len < (sizeof(u->u.msg) + u->u.msg.len))
		goto out;	/* incomplete data portion */

	/*
	 * OK, now we have a complete message.  Do something with it.
	 */

	msg_type = u->u.msg.type;

	switch (msg_type) {
	case XS_TRANSACTION_START:
	case XS_TRANSACTION_END:
	case XS_DIRECTORY:
	case XS_READ:
	case XS_GET_PERMS:
	case XS_RELEASE:
	case XS_GET_DOMAIN_PATH:
	case XS_WRITE:
	case XS_MKDIR:
	case XS_RM:
	case XS_SET_PERMS:
		/* Send out a transaction */
		ret = xenbus_write_transaction(msg_type, u);
		break;

	case XS_WATCH:
	case XS_UNWATCH:
		/* (Un)Ask for some path to be watched for changes */
		ret = xenbus_write_watch(msg_type, u);
		break;

	default:
		ret = -EINVAL;
		break;
	}
	if (ret != 0)
		rc = ret;

	/* Buffered message consumed */
	u->len = 0;

 out:
	mutex_unlock(&u->msgbuffer_mutex);
	return rc;
}


Jun Zhu
Citrix Systems UK
________________________________________
发件人: Patrick Colp [pjcolp@cs.ubc.ca]
发送时间: 2010年8月31日 下午 1:51
收件人: Ian Campbell
抄送: Jun Zhu (Intern); Tim Deegan; dgdegra@tycho.nsa.gov; xen-devel@lists.xensource.com
主题: Re: [Xen-devel] Re: [PATCH] libxl: fix xenstore connection when run in domU

Which XenBus driver is this? In Linux? I've certainly had no issues
running a Mini-OS XenStore stubdom (at least not in regards to missing
things like XS_INTRODUCE).


Patrick


On 31 August 2010 03:44, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> On Tue, 2010-08-31 at 11:32 +0100, Jun Zhu (Intern) wrote:
>>
>> If xenstore runs in a seperate domain, it must use xenbus dev to
>> communicate. If so, the xenstore commands in Xenbus driver are not
>> complete. For example, it does not contain XS_INTRODUCE now.
>
> IIRC Diego's original xenstore-stubdom patch series from way back when
> (posted to the list by Diego and again later by Alex Zeffertt) included
> patches to the kernel side xenbus driver to resolve issues like this.
>
> Ian.
>
>
>

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: ????: Re: [PATCH] libxl: fix xenstore connection when run in domU

[Tim Deegan]

At 09:40 +0100 on 01 Sep (1283334016), Jun Zhu (Intern) wrote:
> In pvops, in the function  xenbus_file_write of file drivers/xen/xenfs/xenbus.c.
> In the switch, there is no XS_INTRODUCE which is used to create a new domain.
> These commands are eliminated here, since in default, a Domu cannot use these commands. 

As Ian Campbell just said, you need to port changesets 874 and 876 of
the linux-2.6.18-xen tree to your pvops kernel:

http://xenbits.xensource.com/linux-2.6.18-xen.hg?rev=876

Cheers,

Tim.

> ??????: Patrick Colp [pjcolp@cs.ubc.ca]
> ????????: 2010??8??31?? ???? 1:51
> ??????: Ian Campbell
> ????: Jun Zhu (Intern); Tim Deegan; dgdegra@tycho.nsa.gov; xen-devel@lists.xensource.com
> ????: Re: [Xen-devel] Re: [PATCH] libxl: fix xenstore connection when run in domU
> 
> Which XenBus driver is this? In Linux? I've certainly had no issues
> running a Mini-OS XenStore stubdom (at least not in regards to missing
> things like XS_INTRODUCE).
> 
> 
> Patrick
> 
> 
> On 31 August 2010 03:44, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> > On Tue, 2010-08-31 at 11:32 +0100, Jun Zhu (Intern) wrote:
> >>
> >> If xenstore runs in a seperate domain, it must use xenbus dev to
> >> communicate. If so, the xenstore commands in Xenbus driver are not
> >> complete. For example, it does not contain XS_INTRODUCE now.
> >
> > IIRC Diego's original xenstore-stubdom patch series from way back when
> > (posted to the list by Diego and again later by Alex Zeffertt) included
> > patches to the kernel side xenbus driver to resolve issues like this.
> >
> > Ian.
> >
> >
> >

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: ????: Re: [PATCH] libxl: fix xenstore connection when run in domU

[Jeremy Fitzhardinge]

 On 09/01/2010 01:52 AM, Tim Deegan wrote:
> At 09:40 +0100 on 01 Sep (1283334016), Jun Zhu (Intern) wrote:
>> In pvops, in the function  xenbus_file_write of file drivers/xen/xenfs/xenbus.c.
>> In the switch, there is no XS_INTRODUCE which is used to create a new domain.
>> These commands are eliminated here, since in default, a Domu cannot use these commands. 
> As Ian Campbell just said, you need to port changesets 874 and 876 of
> the linux-2.6.18-xen tree to your pvops kernel:
>
> http://xenbits.xensource.com/linux-2.6.18-xen.hg?rev=876

876 looks pretty hairy, but I just did 874.

    J