[Xen-devel] [PATCH] sg-run-job: guest-start/repeat: Run 30 times, not 10
We are experiencing intermittent failures right now with the ARM
credit2 tests. I suspect the failure probability is low.
CC: Julien Grall <julien.gr...@arm.com>
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
---
sg-run-job |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sg-run-job b/sg-run-job
index aa97ee6..c66d67d 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -604,7 +604,7 @@ proc test-guest {g} {
proc test-guest-nomigr {g} {
run-ts . = ts-guest-stop+ host $g
-repeat-ts 10 =.repeat \
+repeat-ts 30 =.repeat \
ts-guest-start + host + $g + \; \
ts-guest-stophost $g +
--
1.7.10.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 10/16] SUPPORT.md: Add Debugging, analysis, crash post-portem
George Dunlap writes ("Re: [PATCH 10/16] SUPPORT.md: Add Debugging, analysis,
crash post-portem"):
> gdbsx security support: Someone may want to debug an untrusted guest,
> so I think we should say 'yes' here.
I think running gdb on an potentially hostile program is foolish.
> I don't have a strong opinion on gdbsx; I'd call it 'supported', but if
> you think we need to exclude it from security support I'm happy with
> that as well.
gdbsx itself is probably simple enough to be fine but I would rather
not call it security supported because that might encourage people to
use it with gdb.
If someone wants to use gdbsx with something that's not gdb then they
might want to ask us to revisit that.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/16] SUPPORT.md: Add some x86 features
Jan Beulich writes ("Re: [PATCH 03/16] SUPPORT.md: Add some x86 features"):
> Much depends on whether you think "guest" == "DomU". To me
> Dom0 is a guest, too.
Not to me. I'm with George. (As far as I can make out his message,
which I think was sent with HTML-style quoting which some Citrix thing
has stripped out, so I can't see who said what.)
But I don't think this is important and I would like to see this
document go in.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [xen-4.6-testing test] 116250: regressions - FAIL
osstest service owner writes ("[xen-4.6-testing test] 116250: regressions -
FAIL"):
> flight 116250 xen-4.6-testing real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/116250/
>
> Regressions :-(
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
> test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stopfail REGR. vs.
> 115190
>
...
> version targeted for testing:
> xen 9b0c2a223132a07f06f0be8e85da390defe998f5
Force pushed.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [xen-4.5-testing test] 116245: regressions - FAIL
osstest service owner writes ("[xen-4.5-testing test] 116245: regressions -
FAIL"):
> flight 116245 xen-4.5-testing real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/116245/
>
> Regressions :-(
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
> test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail REGR. vs.
> 115226
> test-amd64-i386-xl-qemuu-ws16-amd64 16 guest-localmigrate/x10 fail in 116223
> REGR. vs. 115226
...
> version targeted for testing:
> xen 41f6dd05d10fd1b4281c1722e2d8f29e378abe9a
Force pushed.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [xen-4.6-testing test] 116222: regressions - FAIL
osstest service owner writes ("[xen-4.6-testing test] 116222: regressions -
FAIL"):
> flight 116222 xen-4.6-testing real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/116222/
>
> Regressions :-(
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
> test-xtf-amd64-amd64-3 49 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs.
> 115190
> test-xtf-amd64-amd64-1 49 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs.
> 115190
Are these somehow expected ? If this is a host-specific expected
regression, then the others
> test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail REGR.
> vs. 115190
> test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stopfail REGR. vs.
> 115190
are just the known Windows problem, and this should be forced.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [xen-4.8-testing test] 116221: regressions - FAIL
osstest service owner writes ("[xen-4.8-testing test] 116221: regressions -
FAIL"):
> flight 116221 xen-4.8-testing real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/116221/
>
> Regressions :-(
...
> version targeted for testing:
> xen 9ba6783e47db71379c5120039b878f605bdf31f3
> baseline version:
> xen 03af24c35ed38967ab8151fdb53da3f6f6cc0872
Force pushed.
> Tests which did not succeed and are blocking,
> including tests which could not be run:
> test-amd64-i386-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail REGR. vs.
> 115205
> test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail REGR.
> vs. 115205
> test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail REGR. vs.
> 115205
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [BUG] Error applying XSA240 update 5 on 4.8 and 4.9 (patch 3 references CONFIG_PV_LINEAR_PT, 3285e75dea89, x86/mm: Make PV linear pagetables optional)
George Dunlap writes ("Re: [BUG] Error applying XSA240 update 5 on 4.8 and 4.9
(patch 3 references CONFIG_PV_LINEAR_PT, 3285e75dea89, x86/mm: Make PV linear
pagetables optional)"):
> These are two different things. Steve's reluctance to backport a
> potentially arbitrary number of non-security-related patches is
> completely reasonable.
I think the right thing to do is this:
If the patche(s) in an XSA require commits from staging-N which are
not contained in previous XSAs, the prerequisite commits should be
listed in the advisory.
That way someone who is following the XSAs (and by implication does
not want to take the other stuff from staging-N/stable-N or even our
point releases) will be able to take the minimum set necessary.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH] ts-xen-build-prep: Install libelf-dev for benefit of linux.git
Juergen Gross writes ("Re: [OSSTEST PATCH] ts-xen-build-prep: Install
libelf-dev for benefit of linux.git"):
> The kernel now is using objtool to create unwind information. This needs
> libelf to work. Advantage is that this approach no longer depends on
> assembler sources being heavily annotated with unwind hints.
Thanks. I have adopted that for the commit message.
> Acked-by: Juergen Gross
I will push this now to osstest pretest.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [OSSTEST PATCH] ts-xen-build-prep: Install libelf-dev for benefit of linux.git
Linux upstream has started needing libelf-dev. Without it, recent tip
fails (in our configuration) like this:
Makefile:938: *** "Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y,
please install libelf-dev, libelf-devel or elfutils-libelf-devel". Stop.
It is not clear exactly when this requirement was introduced. Our
bisector said:
Bug introduced: 91a6a6cfee8ad34ea4cc10a54c0765edfe437cdb
Bug not present: 1c9dbd4615fd751e5e0b99807a3c7c8612e28e20
but the "introduced" commit is a merge of a large branch, so it's not
blaming a specific commit. None of the commits in that range mention
libelf so the most likely reason is a consequence of a change to some
configuration interactions (ie, probably, an expansion of the scope of
an existing dependency).
CC: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
CC: Stefano Stabellini <sstabell...@kernel.org>
CC: Boris Ostrovsky <boris.ostrov...@oracle.com>
CC: Juergen Gross <jgr...@suse.com>
CC: Paul Durrant <paul.durr...@citrix.com>
CC: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
---
ts-xen-build-prep | 1 +
1 file changed, 1 insertion(+)
diff --git a/ts-xen-build-prep b/ts-xen-build-prep
index 3e98364..3309216 100755
--- a/ts-xen-build-prep
+++ b/ts-xen-build-prep
@@ -207,6 +207,7 @@ sub prep () {
autoconf automake libtool xsltproc
libxml2-utils libxml2-dev
libdevmapper-dev w3c-dtd-xhtml libxml-xpath-perl
+ libelf-dev
ccache nasm checkpolicy ebtables);
if ($ho->{Suite} !~ m/squeeze|wheezy/) {
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] tools: xentoolcore_restrict_all: Do deregistration before close
Ross Lagerwall writes ("Re: [PATCH] tools: xentoolcore_restrict_all: Do
deregistration before close"):
> On 11/14/2017 12:15 PM, Ian Jackson wrote:
> > + * Note for multi-threaded programs: If xentoolcore_restrict_all is
> > + * called concurrently with a function which /or closes Xen library
>
> "which /or closes..." - Is this a typo?
Yes, fixed, thanks.
> > -close(h->fd);
> > xentoolcore__deregister_active_handle(>tc_ah);
> > +close(h->fd);
> >
>
> Since the rest of this file uses tabs, you may as well use tabs for this
> line as well.
I didn't change the use of tabs vs. the use of spaces.
> Reviewed-by: Ross Lagerwall <ross.lagerw...@citrix.com>
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10] tools: xentoolcore_restrict_all: Do deregistration before close
Julien Grall writes ("Re: [PATCH] tools: xentoolcore_restrict_all: Do
deregistration before close"):
> I think this is 4.10 material, xentoolcore was introduced in this
> release and it would be good to have it right from now. I want to
> confirm that you are both happy with that?
Yes, absolutely. Sorry, I forgot the for-4.10 tag in the Subject.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] tools: xentoolcore_restrict_all: Do deregistration before close
Closing the fd before unhooking it from the list runs the risk that a
concurrent thread calls xentoolcore_restrict_all will operate on the
old fd value, which might refer to a new fd by then. So we need to do
it in the other order.
Sadly this weakens the guarantee provided by xentoolcore_restrict_all
slight, but not (I think) in a problematic way. It would be possible
to implement the previous guarantee, but it would involve replacing
all of the close() calls in all of the individual osdep parts of all
of the individual libraries with calls to a new function which does
dup2("/dev/null", thing->fd);
pthread_mutex_lock(_lock);
thing->fd = -1;
pthread_mutex_unlock(_lock);
close(fd);
which would be terribly tedious.
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
---
tools/libs/call/core.c | 4 ++--
tools/libs/devicemodel/core.c | 4 ++--
tools/libs/evtchn/core.c | 4 ++--
tools/libs/foreignmemory/core.c| 4 ++--
tools/libs/gnttab/gnttab_core.c| 4 ++--
tools/libs/toolcore/include/xentoolcore.h | 9 +
tools/libs/toolcore/include/xentoolcore_internal.h | 6 --
tools/xenstore/xs.c| 4 ++--
8 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/tools/libs/call/core.c b/tools/libs/call/core.c
index b256fce..f3a3400 100644
--- a/tools/libs/call/core.c
+++ b/tools/libs/call/core.c
@@ -59,8 +59,8 @@ xencall_handle *xencall_open(xentoollog_logger *logger,
unsigned open_flags)
return xcall;
err:
-osdep_xencall_close(xcall);
xentoolcore__deregister_active_handle(>tc_ah);
+osdep_xencall_close(xcall);
xtl_logger_destroy(xcall->logger_tofree);
free(xcall);
return NULL;
@@ -73,8 +73,8 @@ int xencall_close(xencall_handle *xcall)
if ( !xcall )
return 0;
-rc = osdep_xencall_close(xcall);
xentoolcore__deregister_active_handle(>tc_ah);
+rc = osdep_xencall_close(xcall);
buffer_release_cache(xcall);
xtl_logger_destroy(xcall->logger_tofree);
free(xcall);
diff --git a/tools/libs/devicemodel/core.c b/tools/libs/devicemodel/core.c
index b66d4f9..355b7de 100644
--- a/tools/libs/devicemodel/core.c
+++ b/tools/libs/devicemodel/core.c
@@ -68,8 +68,8 @@ xendevicemodel_handle *xendevicemodel_open(xentoollog_logger
*logger,
err:
xtl_logger_destroy(dmod->logger_tofree);
-xencall_close(dmod->xcall);
xentoolcore__deregister_active_handle(>tc_ah);
+xencall_close(dmod->xcall);
free(dmod);
return NULL;
}
@@ -83,8 +83,8 @@ int xendevicemodel_close(xendevicemodel_handle *dmod)
rc = osdep_xendevicemodel_close(dmod);
-xencall_close(dmod->xcall);
xentoolcore__deregister_active_handle(>tc_ah);
+xencall_close(dmod->xcall);
xtl_logger_destroy(dmod->logger_tofree);
free(dmod);
return rc;
diff --git a/tools/libs/evtchn/core.c b/tools/libs/evtchn/core.c
index 2dba58b..aff6ecf 100644
--- a/tools/libs/evtchn/core.c
+++ b/tools/libs/evtchn/core.c
@@ -55,8 +55,8 @@ xenevtchn_handle *xenevtchn_open(xentoollog_logger *logger,
unsigned open_flags)
return xce;
err:
-osdep_evtchn_close(xce);
xentoolcore__deregister_active_handle(>tc_ah);
+osdep_evtchn_close(xce);
xtl_logger_destroy(xce->logger_tofree);
free(xce);
return NULL;
@@ -69,8 +69,8 @@ int xenevtchn_close(xenevtchn_handle *xce)
if ( !xce )
return 0;
-rc = osdep_evtchn_close(xce);
xentoolcore__deregister_active_handle(>tc_ah);
+rc = osdep_evtchn_close(xce);
xtl_logger_destroy(xce->logger_tofree);
free(xce);
return rc;
diff --git a/tools/libs/foreignmemory/core.c b/tools/libs/foreignmemory/core.c
index 79b24d2..7c8562a 100644
--- a/tools/libs/foreignmemory/core.c
+++ b/tools/libs/foreignmemory/core.c
@@ -57,8 +57,8 @@ xenforeignmemory_handle
*xenforeignmemory_open(xentoollog_logger *logger,
return fmem;
err:
-osdep_xenforeignmemory_close(fmem);
xentoolcore__deregister_active_handle(>tc_ah);
+osdep_xenforeignmemory_close(fmem);
xtl_logger_destroy(fmem->logger_tofree);
free(fmem);
return NULL;
@@ -71,8 +71,8 @@ int xenforeignmemory_close(xenforeignmemory_handle *fmem)
if ( !fmem )
return 0;
-rc = osdep_xenforeignmemory_close(fmem);
xentoolcore__deregister_active_handle(>tc_ah);
+rc = osdep_xenforeignmemory_close(fmem);
xtl_logger_destroy(fmem->logger_tofree);
free(fmem);
return rc;
diff --git a/tools/libs/gnttab/gnttab_core.c b/tools/libs/gnttab/gnttab_core.c
index 5f761e5..98f1591 100644
--- a/tools/libs/gnttab/gnttab_core.c
+++ b/tools/libs/gnttab/gnttab_core.c
@@ -54,8 +54,8 @@ xengnttab_handle *xengnttab_open(xentoollog_logger *logger,
unsigned open_flags)
return xgt;
err:
-osdep_gnttab_close(xgt
Re: [Xen-devel] [PATCH for-4.10] libs/evtchn: Remove active handler on clean-up or failure
Ross Lagerwall writes ("Re: [PATCH for-4.10] libs/evtchn: Remove active handler
on clean-up or failure"):
> Now that I look at it, a similar scenario can happen during open. Since
> the handle is registered before it is actually opened, a concurrent
> xentoolcore_restrict_all() will try to restrict a handle that it not
> properly set up.
I think this is not a problem because the handle has thing->fd = -1.
So the restrict call will be a no-op (or give EBADF).
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10] libs/evtchn: Remove active handler on clean-up or failure
Ross Lagerwall writes ("Re: [PATCH for-4.10] libs/evtchn: Remove active handler
on clean-up or failure"):
> On 11/10/2017 05:10 PM, Julien Grall wrote:
> > Commit 89d55473ed16543044a31d1e0d4660cf5a3f49df "xentoolcore_restrict_all:
> > Implement for libxenevtchn" added a call to register allowing to
> > restrict the event channel.
> >
> > However, the call to deregister the handler was not performed if open
> > failed or when closing the event channel. This will result to corrupt
> > the list of handlers and potentially crash the application later one.
Sorry for not spotting this during review.
The fix is correct as far as it goes, so:
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
> > The call to xentoolcore_deregister_active_handle is done at the same
> > place as for the grants. But I am not convinced this is thread safe as
> > there are potential race between close the event channel and restict
> > handler. Do we care about that?
...
> However, I think it should call xentoolcore__deregister_active_handle()
> _before_ calling osdep_evtchn_close() to avoid trying to restrict a
> closed fd or some other fd that happens to have the same number.
You are right. But this slightly weakens the guarantee provided by
xentoolcore_restrict_all.
> I think all the other libs need to be fixed as well, unless there was a
> reason it was done this way.
I will send a further patch. In the meantime I suggest we apply
Julien's fix.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1] tools/hotplug: convert proc-xen.mount to proc-xen.service
Olaf Hering writes ("Re: [Xen-devel] [PATCH v1] tools/hotplug: convert
proc-xen.mount to proc-xen.service"):
> On Wed, Nov 08, Wei Liu wrote:
> > But is there really no way to ask nicely to see if systemd would accept
> > a change in behaviour? That is, to make proc-xen.mount (or any attempt
> > to mount API fs) a nop when xenfs is added to API file system.
>
> I have considered that as well. If the failing unit is "proc-xen.mount"
> and /proc/xen exists, just ignore the error. I will check if and how
> that can be done.
It seems to me that this should be the case for all mount units.
Nothing here is Xen-specific, apart from the particular details of the
consequential lossage.
Ie, mount units should be idempotent even if something else has
already mounted them. Certainly mount units should be a no-op if
their functionality has been moved into the systemd internal mount
table.
Can we please see what systemd upstream think of that proposal ?
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] Bringing up OSS test framework on moonshot(aarch64) systems
Julien Grall writes ("Re: Bringing up OSS test framework on moonshot(aarch64)
systems"):
> On 08/11/17 11:39, Ian Jackson wrote:
> > I'm not familiar with the referent of "moonshot" in this context. IME
> > "moonshot" is a project name chosen multiple times, for different
> > projects, by people who want to give an impression that the project is
> > ambitious.
>
> In that context, this is an moonshot brand from HP. It is a 4.3U that
> accepts up to 45 cartridges (see [1]).
Aha.
> They have x86 cartridges but also provides Arm64 one based on X-Gene 1
> (APM).
>
> Bhupinder is looking at bring up Osstest on the Arm64 cartridges.
Thanks for the explanation. I'm happy to help.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [linux-linus test] 115643: regressions - FAIL
osstest service owner writes ("[linux-linus test] 115643: regressions - FAIL"):
> flight 115643 linux-linus real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/115643/
>
> Regressions :-(
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
> test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail REGR. vs.
> 114682
...
> version targeted for testing:
> linuxe4880bc5dfb1f02b152e62a894b5c6f3e995b3cf
Forced.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [linux-4.9 test] 115504: regressions - FAIL
Roger Pau Monné writes ("Re: [Xen-devel] [linux-4.9 test] 115504: regressions -
FAIL"):
> On Fri, Nov 03, 2017 at 08:21:31PM +, osstest service owner wrote:
> > flight 115504 linux-4.9 real [real]
> > http://logs.test-lab.xenproject.org/osstest/logs/115504/
> >
> > Regressions :-(
> >
> > Tests which did not succeed and are blocking,
> > including tests which could not be run:
> > test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail REGR. vs.
> > 114814
>
> AFAICT this tree should also be force-pushed, the windows 16 issue is
> the same as the one seen on xen-unstable.
Yes, I did that on Monday. This flight was already in progress by
then.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] Bringing up OSS test framework on moonshot(aarch64) systems
Bhupinder Thakur writes ("Bringing up OSS test framework on moonshot(aarch64)
systems"):
> While going through [1], I have some queries/doubts on the configuration.
>
> 1. The following configuration:
>
> DnsDomain uk.xensource.com
> NetNameservers 10.80.248.2 10.80.16.28 10.80.16.67
> HostProp_DhcpWatchMethod leases dhcp3 dhcp.uk.xensource.com:5556
> TftpPath /usr/groups/netboot/
>
> DebianNonfreeFirmware firmware-bnx2
> DebianSuite squeeze
> DebianMirrorHost debian.uk.xensource.com
> DebianPreseed= < <‘END’
> d-i clock-setup/ntp-server string ntp.uk.xensource.com
> END
>
> 1. In this configuration, where would the DNS server be running? Does
> it expect that a DNS server is already configured in the network and
> it has mapping of name <--> IP address for all test hosts? Or do we
> need to setup it up on the OSS controller?
The information about the nameservers, the tftp server, and the ntp
server, is supposed to refer to infrastructure that already exists.
I think your test hosts should be in the DNS, yes. It may be possible
to get it to work without doing that but I wouldn't recommend it.
osstest does not need a dedicated network. Specifically, it can
share its broadcast domain, and its dhcp and tftp servers (and web
proxies, Debian mirrors, ntp servers, and so on), with other uses.
When running osstest in production ("Executive") mode the individual
test boxes must be configured to be available to osstest only if they
are not being used for something else, of course.
See INSTALL.production.
> 2. What is the DhcpWatchMethod option used for?
See under DHCP in INSTALL.production, and please let me know if that's
not clear.
> 3. How are the debian related options mentioned above used? Does OSS
> fetches the installers/preseed files from DEbianMirrorHost and place
> them in the required tftp folders?
mg-debian-installer-update downloads d-i installation information and
puts it in the tftp area.
But the tftp area is also updated at runtime, obviously, in order to
control the booting of each host. And the mirror host is accessed
separately, too.
> I may have more doubts as I try to set things up.
I'm happy to answer more questions, of course :-).
> [1]
> https://blog.xenproject.org/2013/09/30/osstest-standalone-mode-step-by-step/
That blog post may be rather out of date, I'm afraid. But the in-tree
documentation is somewhat better since then.
> I am trying to bring up OSS test framework on a couple of moonshot
> systems which are accessible to me remotely.
I'm not familiar with the referent of "moonshot" in this context. IME
"moonshot" is a project name chosen multiple times, for different
projects, by people who want to give an impression that the project is
ambitious.
Regards,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH 7/8] os-posix: Provide new -runasid option
Markus Armbruster writes ("Re: [Qemu-devel] [PATCH 7/8] os-posix: Provide new
-runasid option"):
> Ian Jackson <ian.jack...@eu.citrix.com> writes:
> > qemu_strtoul fails (returns an error) if the delimiter (that is, the
> > first character which is not processed as digit by strtoul) is not
> > '\0'.
>
> It does that *only* when its @endptr argument is null. Since you pass
> , it should work fine here.
I have just read it again and you are right. Sorry. I will fix this
then.
> > Does that all make sense ?
>
> Your code makes sense to me. It's just the comment that confuses me.
> Does ID mean "implementation-defined behavior"? That would be wrong:
Yes, that's what I meant by ID.
>6.3.1.3 Signed and unsigned integers
>
>[#1] When a value with integer type is converted to another
>integer type other than _Bool, if the value can be
>represented by the new type, it is unchanged.
>
>[#2] Otherwise, if the new type is unsigned, the value is
>converted by repeatedly adding or subtracting one more than
>the maximum value that can be represented in the new type
>until the value is in the range of the new type.
That applies if the new type (uid_t, here) is unsigned. But I think
uid_t's signedness is not specified, so it might be signed. (SuS
says, in the section on types.h, only
Additionally:
* nlink_t, uid_t, gid_t, and id_t shall be integer types.
C99 6.3.1.3 continues:
Otherwise, the new type is signed and the value cannot be
represented in it; either the result is
implementation-defined or an implementation-defined signal is
raised.
So the type of uid_t is ID too.
> One more thing: please report errors with error_report(). Here:
> error_report("Could not obtain uid");
>
> No need to quote optarg back at the user, because error_report() already
> does.
Ah, I will do that. Thanks.
Regards,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [OSSTEST PATCH 1/2] mg-force-push: New script
This does some safety checks and reduces the risk of c mistakes.
It has to be run as osst...@osstest.test-lab (or equivalent).
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
---
mg-force-push | 121 ++
1 file changed, 121 insertions(+)
create mode 100755 mg-force-push
diff --git a/mg-force-push b/mg-force-push
new file mode 100755
index 000..19f99ad
--- /dev/null
+++ b/mg-force-push
@@ -0,0 +1,121 @@
+#!/usr/bin/perl -w
+#
+# usage:
+# ./mg-force-push BRANCH FLIGHT REVISION
+#
+# works only if BRANCH is
+# valid for ap-fetch and ap-print-url
+# the branch of flight FLIGHT
+
+use strict;
+use warnings;
+
+use Osstest;
+
+my @dryrun;
+
+while (@ARGV && $ARGV[0] =~ m/^-/) {
+$_ = shift @ARGV;
+last if $_ eq '-';
+while (m/^-./) {
+if (s/^-n/-/ || s/^--dry-run$//) {
+push @dryrun, qw(echo);
+} else {
+die "$_ ?";
+}
+}
+}
+
+die unless @ARGV==3;
+
+our ($branch, $flight, $revision) = @ARGV;
+
+csreadconfig();
+
+our $url;
+
+sub db_checks () {
+my $flt_q = $dbh_tests->prepare(<<END);
+SELECT branch, blessing
+ FROM flights
+ WHERE flight=?
+END
+my $rev_q = $dbh_tests->prepare(<<END);
+WITH rv AS (
+SELECT *
+ FROM runvars
+WHERE flight=?
+ )
+SELECT
+ url.jobjob,
+ built.name bname,
+ built.val brevision
+FROM rv url
+JOIN rv built
+ ON url.job= built.job
+AND url.name like 'tree_%'
+AND built.name = 'built_revision_' || substring(url.name, 6)
+ WHERE url.val = ?
+END
+
+my %revuses;
+printf "checking revisioins used in %s...\n", $flight;
+
+db_readonly_report();
+db_retry($dbh_tests, [], sub {
+$flt_q->execute($flight);
+my $flt_row = $flt_q->fetchrow_hashref();
+$flt_row->{blessing} eq 'real' or die "$flt_row->{blessing} ?";
+$flt_row->{branch} eq $branch or die "$flt_row->{branch} ?";
+
+%revuses = ();
+$rev_q->execute($flight, $url);
+while (my $rev_row = $rev_q->fetchrow_hashref()) {
+push @{ $revuses{ $rev_row->{brevision} } }, $rev_row;
+}
+});
+
+die unless $revuses{$revision};
+my $bad;
+foreach my $brevision (sort { @{ $revuses{$b} } <=>
+ @{ $revuses{$a} } } keys %revuses) {
+my $rj = $revuses{$brevision};
+printf " %s%s %d uses\n", $brevision,
+($brevision eq $revision ? ' (ours)' : ''),
+scalar @$rj;
+foreach my $use (@$rj) {
+printf "%-30s %s\n", $use->{job}, $use->{bname};
+}
+if (@$rj > @{ $revuses{$revision} }) {
+printf " ^^ more popular!\n";
+$bad = 1;
+}
+}
+die "our revision $revision is not most popular in $flight !" if $bad;
+}
+
+sub geturl () {
+$!=0; $?=0; $url = `./ap-print-url $branch`;
+die "$? $!" unless chomp $url;
+printf "tree in flights should be %s\n", $url;
+}
+
+sub runcmd_ordryrun {
+$!=0; $?=0;
+print "@_\n" unless @dryrun;
+system((@dryrun, @_))
+and die "@dryrun @_ $! $?" if $! or $:
+}
+
+sub fetch () {
+runcmd_ordryrun qw(./ap-fetch-version), $branch;
+}
+
+sub dopush () {
+runcmd_ordryrun qw(./ap-push), $branch, $revision;
+}
+
+geturl();
+db_checks();
+fetch();
+dopush();
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [OSSTEST PATCH 2/2] ap-push: turn off set -x
This makes the output of mg-force-push quite unpleasant, amongst other things. Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com> --- ap-push | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ap-push b/ap-push index a27ccc2..6c95b1f 100755 --- a/ap-push +++ b/ap-push @@ -17,7 +17,7 @@ # along with this program. If not, see <http://www.gnu.org/licenses/>. -set -ex -o posix +set -e -o posix branch=$1 revision=$2 -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [linux-4.9 test] 115538: regressions - FAIL
osstest service owner writes ("[linux-4.9 test] 115538: regressions - FAIL"):
> flight 115538 linux-4.9 real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/115538/
>
> Regressions :-(
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
> test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail REGR. vs.
> 114814
...>
> version targeted for testing:
> linux06b639e5a1a665ba6c959398ea0e6171c162028b
(test-lab)osstest@osstest:/home/iwj/testing.git$
OSSTEST_CONFIG=production-config ./mg-force-push linux-4.9 115538
06b639e5a1a665ba6c959398ea0e6171c162028b
tree in flights should be
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
checking revisioins used in 115538...
06b639e5a1a665ba6c959398ea0e6171c162028b (ours) 3 uses
build-amd64-pvops built_revision_linux
build-armhf-pvops built_revision_linux
build-i386-pvops built_revision_linux
./ap-fetch-version linux-4.9
06b639e5a1a665ba6c959398ea0e6171c162028b
./ap-push linux-4.9 06b639e5a1a665ba6c959398ea0e6171c162028b
...
To osst...@xenbits.xen.org:/home/xen/git/linux-pvops.git
5d7a76a..06b639e 06b639e5a1a665ba6c959398ea0e6171c162028b ->
tested/linux-4.9
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH 7/8] os-posix: Provide new -runasid option
Hi. Thanks for the (re)-review.
Markus Armbruster writes ("Re: [Qemu-devel] [PATCH 7/8] os-posix: Provide new
-runasid option"):
> Ian Jackson <ian.jack...@eu.citrix.com> writes:
> > +case QEMU_OPTION_runasid:
> > +errno = 0;
> > +lv = strtoul(optarg, , 0); /* can't qemu_strtoul, want *ep=='.'
> > */
>
> I'm afraid I can't see why qemu_strtoul() wouldn't work here. Can you
> enlighten me?
qemu_strtoul fails (returns an error) if the delimiter (that is, the
first character which is not processed as digit by strtoul) is not
'\0'. Normally this is desirable, because it correctly rejects
strings like "123blather". But here we are trying to process a string
whose first non-digit is ':', and we will handle the tail part
explicitly.
It would be possible to use strchr and then to write a '\0' over the
':' but I dislike that processing style (and it is forbidden by the
const annotations on os_parse_cmd_args etc.)
> > +user_uid = lv; /* overflow here is ID in C99 */
>
> I don't get the comment. You check for overflow the obvious way below.
> Sure you need a comment?
This relates to overflow in the assignment, only. lv is an unsigned
long and user_uid is a uid_t (which may be smaller). Normally, signed
integer overflow is UB, but this is not the case when converting from
another integer type.
There are two possible overflows: 1. a string that strtoul cannot get
to fit in an unsigned long, which produces a nonzero errno; and, 2. a
value which fits in an unsigned long but not a uid_t. In the latter
case, we convert it _back_ into an unsigned long, as an implicit
conversion in this middle test:
> > +if (errno || *ep != '.' || user_uid != lv || user_uid ==
> > (uid_t)-1) {
If that succeeds, we know we can round-trip it so it is valid. The
remaining check needed is that it doesn't round trip to the sentinel
uid value.
Does that all make sense ? I'm not sure how much of this to document
in comments. It's deeply annoying that C is such a puzzle language
here and that therefore such complicated reasoning and
not-immediately-obvious code is needed, to do something so simple.
If you would like me to remove the comment, I can drop it, of course.
I don't really mind.
> > +#ifndef _WIN32
> > +DEF("runasid", HAS_ARG, QEMU_OPTION_runasid, \
> > +"-runasid uid.gid change to numeric uid and gid just before
> > starting the VM\n",
> > +QEMU_ARCH_ALL)
> > +#endif
> > +STEXI
> > +@item -runasid @var{uid}.@var{gid}
>
> Didn't we agree on using ':' instead of '.' as separator?
>
> Sure we need yet another option? Why can't we compatibly extend -runas?
For some reason you are looking at an old version of the patch. That
may be my fault - there were a few mis-posts. Sorry about that.
The final version does indeed have ':' and does reuse the -runas
option.
> If we truly need both, the rationale belongs into the commit message,
> and we need to consider how they interact. I'd recommend left-to-right
> semantics, i.e. if you specify both, the last one wins. Not what your
> current code does, if I read it correctly.
Happily this is now irrelevant.
I will repost this series after I hear from you about strtoul and the
overflow comment.
Regards,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-next 1/9] gcov: return ENOSYS for unimplemented gcov domctl
Jan Beulich writes ("Re: [PATCH for-next 1/9] gcov: return ENOSYS for
unimplemented gcov domctl"):
> On 26.10.17 at 11:19, wrote:
> > --- a/xen/common/gcov/gcov.c
> > +++ b/xen/common/gcov/gcov.c
> > @@ -239,7 +239,7 @@ int sysctl_gcov_op(struct xen_sysctl_gcov_op *op)
> > break;
> >
> > default:
> > -ret = -EINVAL;
> > +ret = -ENOSYS;
> > break;
> > }
>
> Very certainly ENOSYS is not in any way better. Despite the many
> misuses of it, we've started enforcing that this wouldn't be spread.
> -EOPNOTSUPP may be fine here, but -EINVAL is suitable as well.
> -ENOSYS exclusively means that a _top level_ hypercall is
> unimplemented (i.e. with very few exceptions there should be
> exactly one place where it gets returned, which is in the main
> hypercall dispatch code).
The distinction between unimplemented status of a top-level hypercall
and unimplemented status of a sub-op is rarely useful to the caller.
Conversely, the distinction between an unimplemented facility, and a
facility which is exists but is being used improperly, is vitally
important to anyone who is trying to write compatibility code.
I don't mind if you want to insist on the former distinction,
reserving ENOSYS for top-level hypercalls and EOPNOTSUPP for other
functions.
But I absolutely do mind the use of EINVAL for "unsupported function".
I appreciate that much of the hypervisor has historically used EINVAL
this way, but this is (a) a pain for callers (b) evil, bad, and wrong
(c) unnecessary since EOPNOTSUPP is available. We should at least not
perpetrate any more of this. In an unreleased API we should change it
before release.
Thanks for your attention.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 2/2] make-flight: guest should use jessie to test pvgrub
Wei Liu writes ("[OSSTEST PATCH 2/2] make-flight: guest should use jessie to
test pvgrub"):
> Stretch has 64bit feature enabled for ext4, which pvgrub can't cope.
> We want to continue to test pvgrub, so specify jessie in the guest
> suite field.
I'm not entirely comfortable with the hardcoding here, but I guess few
people are still using osstest configured to use squeeze so I think it
will do.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] Commit moratorium to staging
George Dunlap writes ("Re: [Xen-devel] Commit moratorium to staging"):
> Well, with a looping xen-build going on in the guest, I've done 40 local
> migrates with no problems yet.
>
> But Roger -- is this on emulated devices only, no PV drivers?
Yes. None of our Windows tests have PV drivers.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] osstest: fix rm to use '-f' in ts-freebsd-host-install
Roger Pau Monne writes ("[PATCH v2] osstest: fix rm to use '-f' in
ts-freebsd-host-install"):
> It's perfectly valid for the .tmp file to not exists, and the script
> shouldn't fail in that case.
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] osstest: remove unneeded rm in ts-freebsd-host-install
Roger Pau Monne writes ("[PATCH] osstest: remove unneeded rm in
ts-freebsd-host-install"):
> The usage of `rm` here is wrong for two reasons:
>
> - It will fail if $sharedpath.tmp doesn't exist and report and error
>(ie: -f should be used).
> - It's not needed because dd will truncate $sharedpath.tmp.
I think it would be marginally better to use -f. That way the
permissions and ownership of a previous abortive attempt are discarded
rather than kept.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 1/2] ts-debian-di-install: use gho to pick d-i
Wei Liu writes ("[OSSTEST PATCH 1/2] ts-debian-di-install: use gho to pick
d-i"):
> The original code used ho which gave us the host suite, but we wanted
> the guest suite.
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [OSSTEST PATCH] migrations: Do x10 migration 20x instead
We want to keep the old testid or some new failures will be "never
pass".
Roger reports that this change makes the existing host-specific
Windows migration failures fail everywhere, so so things may need
force pushing.
CC: Roger Pau Monné <roger@citrix.com>
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
---
sg-run-job | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sg-run-job b/sg-run-job
index 537d6b8..aa97ee6 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -592,7 +592,7 @@ proc test-guest-migr {g} {
run-ts . =.2 ts-guest-saverestore + host $g
}
if {$can_migrate} {
-run-ts . =/x10 ts-guest-localmigrate + x10 host $g
+run-ts . =/x10 ts-guest-localmigrate + x20 host $g
}
}
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] Commit moratorium to staging [and 1 more messages]
Julien Grall writes ("Re: Commit moratorium to staging"):
> Thank you for the explanation. I agree with the force push to unblock
> master (and other tree I mentioned).
I will force push all the affected trees, but in a reactive way
because I base each force push on a test report - so it won't be right
away for all of them.
osstest service owner writes ("[xen-unstable test] 115471: regressions - FAIL"):
> flight 115471 xen-unstable real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/115471/
>
> Regressions :-(
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
> test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stopfail REGR. vs.
> 114644
> test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail REGR. vs.
> 114644
> test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail REGR. vs.
> 114644
The above are justifiable as discussed, leaving no blockers.
> version targeted for testing:
> xen bb2c1a1cc98a22e2d4c14b18421aa7be6c2adf0d
So I have forced pushed that.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] Commit moratorium to staging
Roger Pau Monné writes ("Re: [Xen-devel] Commit moratorium to staging"):
> Is there anyway to get that from windows in an automatic way? If not I
> could test that with a Debian guest. In fact it might even be a good
> thing for Linux based guest to be added to the regular migration tests
> in order to make sure cpuid bits don't change across migrations.
We do migrations of all the guests in osstest (apart from in ARM,
where the guests don't support it, and some special cases like
rumpkernel and xtf domains).
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] Commit moratorium to staging
Julien Grall writes ("Re: Commit moratorium to staging"):
> Hi Ian,
>
> Thank you for the detailed e-mail.
>
> On 11/01/2017 02:07 PM, Ian Jackson wrote:
> > Furthermore, the test is not intermittent, so a force push will be
> > effective in the following sense: we would only get a "spurious" pass,
> > resulting in the relevant osstest branch becoming stuck again, if a
> > future test was unlucky and got an unaffected host. That will happen
> > infrequently enough.
...
> I am not entirely sure to understand this paragraph. Are you saying that
> osstest will not get stuck if we get a "spurious" pass on some hardware
> in the future? Or will we need another force push?
osstest *would* get stuck *if* we got such a spurious push. However,
because osstest likes to retest failing tests on the same host as they
failed on previously, such spurious passes are fairly unlikely.
I say "likes to". The allocation system uses a set of heuristics to
calculate a score for each possible host. The score takes into
account both when the host will be available to this job, and
information like "did the most recent run of this test, on this host,
pass or fail". So I can't make guarantees but the amount of manual
work to force push stuck branches will be tolerable.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] Commit moratorium to staging
So, investigations (mostly by Roger, and also a bit of archaeology in the osstest db by me) have determined: * This bug is 100% reproducible on affected hosts. The repro is to boot the Windows guest, save/restore it, then migrate it, then shut down. (This is from an IRL conversation with Roger and may not be 100% accurate. Roger, please correct me.) * Affected hosts differ from unaffected hosts according to cpuid. Roger has repro'd the bug on an unaffected host by masking out certain cpuid bits. There are 6 implicated bits and he is working to narrow that down. * It seems likely that this is therefore a real bug. Maybe in Xen and perhaps indeed one that should indeed be a release blocker. * But this is not a regresson between master and staging. It affects many osstest branches apparently equally. * This test is, effectively, new: before the osstest change "HostDiskRoot: bump to 20G", these jobs would always fail earlier and the affected step would not be run. * The passes we got on various osstest branches before were just because those branches hadn't tested on an affected host yet. As branches test different hosts, they will stick on affected hosts. ISTM that this situation would therefore justify a force push. We have established that this bug is very unlikely to be anything to do with the commits currently blocked by the failing pushes. Furthermore, the test is not intermittent, so a force push will be effective in the following sense: we would only get a "spurious" pass, resulting in the relevant osstest branch becoming stuck again, if a future test was unlucky and got an unaffected host. That will happen infrequently enough. So unless anyone objects (and for xen.git#master, with Julien's permission), I intend to force push all affected osstest branches when the test report shows the only blockage is ws16 and/or win10 tests failing the "guest-stop" step. Opinions ? Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 for-4.10] scripts: introduce a script for build test
Wei Liu writes ("[PATCH v3 for-4.10] scripts: introduce a script for build
test"):
> Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
> Signed-off-by: Wei Liu <wei.l...@citrix.com>
...
...
> +trap "echo Restoring original HEAD ; git checkout $ORIG_BRANCH" EXIT
This will smash the whole script's exit status. I think you need to
save/restore $?. Be careful with your quoting. Normally it is better
for the argument to trap to be ''-quoted rather than "", to avoid it
being expanded twice (and, the first time, too soon).
Also, if this fails, it leaves the failure message buried in a scrool
of make -j4 output, where the user probably won't see it. And it
prints exactly the same message on success and failure. On failure
you should print the failing commitid, and exit nonzero.
On success you should print some reassuring `ok' message.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10] tools/hotplug: create XEN_LOG_DIR at runtime
Wei Liu writes ("Re: [PATCH] tools/hotplug: create XEN_LOG_DIR at runtime"):
> On Fri, Oct 27, 2017 at 07:52:37PM +0300, Andrii Anisov wrote:
> > From: Andrii Anisov <andrii_ani...@epam.com>
> >
> > /var/log could be a tmpfs mount point, so create xen subfolder at runtime.
> >
> > Signed-off-by: Andrii Anisov <andrii_ani...@epam.com>
> > Reviewed-by: Volodymyr Babchuk <volodymyr_babc...@epam.com>
> > Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>
>
> Acked-by: Wei Liu <wei.l...@citrix.com>
>
> Julien I think we should apply this for 4.10.
I agree. Subject line tag added.
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 12/16] osstest: add script to install build dependencies on FreeBSD
Roger Pau Monne writes ("[PATCH 12/16] osstest: add script to install build
dependencies on FreeBSD"):
> Since at the moment osstest only builds FreeBSD on FreeBSD, there are
> no dependencies to install. Just mark the host as ready to share.
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 14/16] osstest: add support for FreeBSD buildjobs to sg-run-job
Roger Pau Monne writes ("[PATCH 14/16] osstest: add support for FreeBSD
buildjobs to sg-run-job"):
> Add support and introduce a FreeBSD build job to sg-run-job.
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 04/16] osstest: introduce host_shared_mark_ready
Roger Pau Monne writes ("[PATCH 04/16] osstest: introduce
host_shared_mark_ready"):
> That allows marking a host as ready to be shared. Replace the current
> callers that open-code it.
>
> Signed-off-by: Roger Pau Monné <roger@citrix.com>
> ---
> Changes since v13:
> - s/resource_shared_mark_ready/host_shared_mark_ready/.
> - First argument of jobdb_resource_shared_mark_ready must be 'host'.
Although it would be good to mention that you are fixing a bug here,
to wit the $ho->{Ident} to 'host' change.
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [OSSTEST PATCH 1/2] resource_shared_mark_ready: Fix for non-`host' idents; fix $sharetype
$ho->{Ident} is completely wrong here. That, ultimately, is going to
be restype. But restype must be the fixed value `host'. If this
function were called with a $ho whose Ident was `src_host' or
something, it would fail because hosts are all restype `host' in the
datanase, of course.
Also `$resource' here is the wrong variable name. This is actually
the $sharetype (as is evident from jobdb_resource_shared_mark_ready
and what is now executive_resource_shared_mark_ready).
CC: Roger Pau Monné <roger@citrix.com>
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
---
Osstest/TestSupport.pm | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 75f5a26..85ed57a 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -2859,10 +2859,10 @@ sub sha256file ($;$) {
}
sub resource_shared_mark_ready($$) {
-my ($ho,$resource) = @_;
+my ($ho,$sharetype) = @_;
-$mjobdb->jobdb_resource_shared_mark_ready($ho->{Ident}, $ho->{Name},
- $resource);
+$mjobdb->jobdb_resource_shared_mark_ready('host', $ho->{Name},
+ $sharetype);
}
1;
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [OSSTEST PATCH 2/2] host_shared_mark_ready: rename from resource_shared_mark_ready
This function only works on resource of restype `host'. Ie, hosts.
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
CC: Roger Pau Monné <roger@citrix.com>
---
Osstest/TestSupport.pm | 4 ++--
ts-freebsd-host-install | 4 ++--
ts-xen-build-prep | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 85ed57a..c9dada3 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -134,7 +134,7 @@ BEGIN {
guest_editconfig_nocd
host_install_postboot_complete
target_core_dump_setup
- sha256file resource_shared_mark_ready
+ sha256file host_shared_mark_ready
);
%EXPORT_TAGS = ( );
@@ -2858,7 +2858,7 @@ sub sha256file ($;$) {
return $truncate ? substr($digest, 0, $truncate) : $digest;
}
-sub resource_shared_mark_ready($$) {
+sub host_shared_mark_ready($$) {
my ($ho,$sharetype) = @_;
$mjobdb->jobdb_resource_shared_mark_ready('host', $ho->{Name},
diff --git a/ts-freebsd-host-install b/ts-freebsd-host-install
index bfc693a..caec993 100755
--- a/ts-freebsd-host-install
+++ b/ts-freebsd-host-install
@@ -279,5 +279,5 @@ setup_netboot_local($ho);
# Proceed with the install
install();
-resource_shared_mark_ready($ho, "build-freebsd-".
-sha256file("$path_prefix/install.img", 16));
+host_shared_mark_ready($ho, "build-freebsd-".
+ sha256file("$path_prefix/install.img", 16));
diff --git a/ts-xen-build-prep b/ts-xen-build-prep
index 776866d..22a3ce6 100755
--- a/ts-xen-build-prep
+++ b/ts-xen-build-prep
@@ -274,4 +274,4 @@ if (!$ho->{Flags}{'no-reinstall'}) {
gitcache_setup();
}
-resource_shared_mark_ready($ho, "build-".$ho->{Suite}."-".$r{arch});
+host_shared_mark_ready($ho, "build-".$ho->{Suite}."-".$r{arch});
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10] tools/xenstored: Check number of strings passed to do_control()
Pawel Wieczorkiewicz writes ("[PATCH] tools/xenstored: Check number of strings
passed to do_control()"):
> It is possible to send a zero-string message body to xenstore's
> XS_CONTROL handling function. Then the number of strings is used
> for an array allocation. This leads to a crash in strcmp() in a
> CONTROL sub-command invocation loop.
> The output of xs_count_string() should be verified and all 0 or
> negative values should be rejected with an EINVAL. At least the
> sub-command name must be specified.
>
> The xenstore crash can only be triggered from within dom0 (there
> is a check in do_control() rejecting all non-dom0 requests with
> an EACCES).
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
(Added the for-4.10 tag to the Subject.)
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 07/20] osstest: introduce resource_shared_mark_ready
Roger Pau Monne writes ("[PATCH v6 07/20] osstest: introduce
resource_shared_mark_ready"):
> That allows marking a host as ready to be shared. Replace the current
> caller that open-codes it.
...
> -$mjobdb->jobdb_resource_shared_mark_ready
> - ($ho->{Ident}, $ho->{Name}, "build-".$ho->{Suite}."-".$r{arch});
> +
Well. On trying to build something on top of this, I notice that
$ho->{Ident} is completely wrong here. That, ultimately, is going to
be restype. But restype must be the fixed value `host'.
> +sub resource_shared_mark_ready($$) {
> +my ($ho,$resource) = @_;
> +
> +$mjobdb->jobdb_resource_shared_mark_ready($ho->{Ident}, $ho->{Name},
> + $resource);
> +}
And this function `resource_shared_mark_ready' only marks hosts ready
- since it takea $ho.
I propose to send you a followup patch which renames your new
resource_shared_mark_ready to host_shared_mark_ready, and passes
'host' instead of $ho->{Ident}. Alternatively I can wait for you to
do this, if that would be disruptive to you.
Also `$resource' here is the wrong variable name. This is actually
the $sharetype (as is evident from jobdb_resource_shared_mark_ready
and what is now executive_resource_shared_mark_ready. That is a bug
which is introduced in your patch. Would you like to respin this as
you are about to rebase this onto what is about to become master,
anyway ?
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10 1/9] gcov: return ENOSYS for unimplemented gcov domctl
Roger Pau Monne writes ("[PATCH for-next 1/9] gcov: return ENOSYS for
unimplemented gcov domctl"):
> Signed-off-by: Roger Pau Monné <roger@citrix.com>
...
> default:
> -ret = -EINVAL;
> +ret = -ENOSYS;
> break;
> }
Reviewed-by: Ian Jackson <ian.jack...@eu.citrix.com>
I think this is a bugfix which should go into 4.10. Julien ?
(Subject line changed by me.)
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.1 8/8] configure: do_compiler: Dump some extra info under bash
Stefano Stabellini writes ("Re: [PATCH v5.1 8/8] configure: do_compiler: Dump
some extra info under bash"):
> CC'ing the maintainers for this.
Thanks, but scripts/get_maintainer.pl seems to print different
information for me... (see my other mail)
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.1 7/8] os-posix: Provide new -runas : facility
Stefano Stabellini writes ("Re: [PATCH v5.1 7/8] os-posix: Provide new -runas
: facility"):
> CC'ing the maintainers (scripts/get_maintainer.pl is your friend)
I don't know what your scripts/get_maintainer.pl does, but mine
says:
get_maintainer.pl: No maintainers found, printing recent contributors.
get_maintainer.pl: Do not blindly cc: them on patches! Use common sense.
Anthony PERARD <anthony.per...@citrix.com> (commit_signer:1/2=50%)
Paolo Bonzini <pbonz...@redhat.com>
(commit_signer:1/2=50%,commit_signer:11/57=19%)
Ian Jackson <ian.jack...@eu.citrix.com> (commit_signer:1/2=50%)
Michael Tokarev <m...@tls.msk.ru> (commit_signer:12/57=21%)
Eric Blake <ebl...@redhat.com> (commit_signer:10/57=18%)
Thomas Huth <th...@redhat.com> (commit_signer:8/57=14%)
Markus Armbruster <arm...@redhat.com> (commit_signer:8/57=14%)
qemu-de...@nongnu.org (open list:POSIX)
I have added Paolo, Markus and Daniel Berrange to the CCs of my patch
on the basis that they have commented already...
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.1 2/8] xen: restrict: use xentoolcore_restrict_all
Stefano Stabellini writes ("Re: [PATCH v5.1 2/8] xen: restrict: use
xentoolcore_restrict_all"):
> On Fri, 20 Oct 2017, Ian Jackson wrote:
...
> > Drop individual use of xendevicemodel_restrict and
> > xenforeignmemory_restrict. These are not actually effective in this
> > version of qemu, because qemu has a large number of fds open onto
> > various Xen control devices.
...
> Wait, if the compat stub returns error, and this patch removed the code
> to check for ENOTTY, doesn't it prevent any QEMU compiled against older
> Xen from working?
>
> Or am I missing something?
You are right, but this is intended. The paragraph I quote in the
commit message above is intended to explain.
That is: without xentoolcore_restrict_all, -xen-domid-restrict is a
booby-trap. It does not actually prevent a compromised qemu from
doing anything. So there is no reason to pass it in such a
configuration. If you do pass it it is better for the domain startup
to fail, than for it to carry on without the restriction.
The only reason I am not saying someone should be issuing an advisory
is that this feature was never supported by any of the Xen toolstacks.
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.1 6/8] xen: destroy_hvm_domain: Try xendevicemodel_shutdown
Stefano Stabellini writes ("Re: [PATCH v5.1 6/8] xen: destroy_hvm_domain: Try
xendevicemodel_shutdown"):
> On Fri, 20 Oct 2017, Ian Jackson wrote:
> > xc_interface_open etc. is not going to work if we have dropped
> > privilege, but xendevicemodel_shutdown will if everything is new
> > enough.
> >
> > xendevicemodel_shutdown is only availabe in Xen 4.10 and later, so
> > provide a stub for earlier versions.
...
> > +if (xen_dmod) {
> > +rc = xendevicemodel_shutdown(xen_dmod, xen_domid, reason);
> > +if (!rc) {
> > +return;
> > +}
> > +perror("xendevicemodel_shutdown failed");
>
> I don't think is a good idea to print an error because this is actually
> a normal condition when QEMU is build and run against an older Xen.
> Users might get confused when looking at the logs.
Oh. Yes. I wrote this before I provided the fallback stub in
xen_common.h, and therefore before I properly understood the approach
taken to fallbacks. The fallback logic here is not correct.
> But it would be correct to print an error if errno != ENOTTY.
Indeed.
I have changed it to read like this:
if (xen_dmod) {
rc = xendevicemodel_shutdown(xen_dmod, xen_domid, reason);
if (!rc) {
return;
}
if (errno != ENOTTY /* old Xen */)
perror("xendevicemodel_shutdown failed");
/* well, try the old thing then */
}
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.1 1/8] xen: link against xentoolcore
Stefano Stabellini writes ("Re: [PATCH v5.1 1/8] xen: link against
xentoolcore"):
> On Fri, 20 Oct 2017, Ian Jackson wrote:
> >then
> > - xen_stable_libs="-lxendevicemodel $xen_stable_libs"
> > + xen_stable_libs="-lxendevicemodel $xen_stable_libs -lxentoolcore"
>
> Is it on purpose that -lxentoolcore is at the end of this string rather
> than before $xen_stable_libs?
Yes. xentoolcore is required by the other libraries, and this is
therefore the correct ordering for situations where the link order
matters.
> In any case
>
> Acked-by: Stefano Stabellini <sstabell...@kernel.org>
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] scripts: introduce a script for build test
Wei Liu writes ("Re: [PATCH v2] scripts: introduce a script for build test"):
> On Wed, Oct 25, 2017 at 04:25:21PM +0100, Ian Jackson wrote:
> > If you are worried about this you should check that there are no
> > uncommitted files before starting.
>
> This is already done in this version.
>
> I don't worry if there is uncommitted file, I just don't want to stop
> developers from being smarter than the script when they know git-clean
> is not necessary.
I don't understand your point. If there are no uncommitted files at
the start of the run, then git clean is certainly safe later, since
everything that will be deleted was made by `make'. Therefore doing
it unconditionally is fine.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] scripts: introduce a script for build test
Wei Liu writes ("Re: [PATCH v2] scripts: introduce a script for build test"):
> On Tue, Oct 24, 2017 at 02:38:39PM +0100, Ian Jackson wrote:
> > Anthony PERARD writes ("Re: [PATCH v2] scripts: introduce a script for
> > build test"):
> > > That feels wrong. How do I run the same exact command at the default
> > > one, but with -j8 instead of -j4?
> >
> > .../build-test sh -ec make -j4 distclean && ./configure && make -j4
> >
> > But I think Anthony has a point. The clean should 1. be git-clean,
> > not make distclean 2. be run anyway.
>
> I don't think we should call git-clean unconditionally -- imagine
> someone knew for sure they only needed to build part of the tools or the
> hypervisor.
If you are worried about this you should check that there are no
uncommitted files before starting.
I have a visceral loathing of clean targets. They are often flaky,
and ours are no exception.
> > > > +echo "Restoring original HEAD"
> > > > +git checkout $ORIG_BRANCH
> > >
> > > Also, what a developper should do when the build fail? She can't modify
> > > the current code, because changes are going to be losts. Maybe we could
> > > trap failures, restore original HEAD and point out which commit fails to
> > > build.
> >
> > IWBNI it would at least print the branch to checkout. Tools like "git
> > bisect" record the information in .git and allow "git bisect reset".
>
> Not sure I follow. Do you want the script to trap SIGCHLD, test the
> return value and act accordingly?
I don't mean you should trap SIGCHLD.
But you should probably trap '' and use it to print a helpful message
containing ORIG_BRANCH. On success you would disable the trap before
exiting.
Alternatively you could defuse `set -e' around the invocation of the
build command, and handle $? explicitly.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() to not pass a structure by value
I have reordered the quoted text, and my replies, so as to address the
most technical points first and leave what might be described as
process arguments and tone complaints for later.
Andrew Cooper writes ("Re: [PATCH for-4.10 1/2] tools/libxc: Fix
precopy_policy() to not pass a structure by value"):
> someone who does understand why hiding a prologue memcpy() is bad for
> performance.
To address your actual technical point about the memcpy.
Not all functions in the toolstack are performance critical and not
all putative tiny performance benefits are worthwhile. Most are not.
Code should generally be written to be as clear and simple as
possible, and clarity and simplicity should be traded off for
performance only when this will produce a noticeable difference.
Obviously clarity is a matter of opinion, but I would generally say
that a struct containing plain data is simpler than a pointer to a
similar struct. And of course passing as a pointer requires (or, in
this case, will eventually require) additional complexity in the
message generator script.
Against that, in this case, the cost of the additional alleged-memcpy
seems to me, at first glance, to be completely irrelevant.
Of course it probably isn't going to be a memcpy; the struct contents
will probably be passed in registers (I haven't double-checked ABI
manuals so this may be wrong on some register-poor architectures).
Given the small size of this struct, it might well be slightly faster
to pass it in a pair of registers rather than a pointer to memory, for
locality of reference reasons.
But ISTM that all of this is going to be swamped by the other costs of
making a function call (at least where the callback is provided -
where it isn't provided, it doesn't matter). And for in-tree
consumers, the cost of the copy-by-value is going to be dwarfed by the
IPC costs (as indeed you notice).
If you have a better argument than "passing a struct by value should
be avoided in all cases for performance reasons" you need to make it.
> Having an IPC call in the middle of the live loop it is bad, and will
> increase the downtime of the VM. However, the IPC call is optional
> which means the common case doesn't need to suffer the overhead.
> Passing by value even in the common case is an entirely unnecessary
> overhead, and this fact alone is justification enough to not do it.
At last, we're starting to get towards a technical argument here.
I think the most significant proportional performance impact would be
the case where there is a callback but only within the migration
process. Ie, an out-of-tree provider of the precopy_policy hook.
(If there is no callback provided, there is no cost; and an in-tree
consumer will have the IPC cost which will dominate.)
Would you care to hazard a guess at the quantifiable peformance loss
from passing this by value, in that case ? Perhaps you would like to
exhibit some assembly snippets, or show a benchmark result.
> However, what is really irritating me is that, not only am I having to
> divert time from more important tasks to try and fix this code before we
> ship a release with it in, but that I'm having to fight you for the
> privilege of maintaining that the migration code doesn't regress back
> into the cesspit that was the legacy code.
Since we are still talking about a libxc interface, there is no
concern from an ABI/API stability point of view. If we decide, later,
that the pointer is better (whether because we have changed our mind,
or because of changed circumstances such as the struct growing
significantly) we can just change it. Of course it's better to get it
right first time so if there is a good reason.
> On 19/10/17 16:17, Ian Jackson wrote:
> > Andrew Cooper writes ("Re: [PATCH for-4.10 1/2] tools/libxc: Fix
> > precopy_policy() to not pass a structure by value"):
> >> On 16/10/17 16:07, Ian Jackson wrote:
> >>> This statement is true only if you think "the precopy callback" refers
> >>> to the stub generated by libxl_save_msgs_gen.
> >> The commit is about save_callbacks.precopy_policy() specifically (and
> >> IMO, obviously).
> >> Given this, the statement is true.
> > I don't agree.
>
> Don't agree with what? The justification for why passing-by-value is
> supposedly necessary is bogus irrespective of whether you consider just
> the libxc part of the callback, or the end-to-end path into libxl.
>
> No argument concerning address space (separate or otherwise) is a
> related to how parameter passing needs to happen at this level.
>
> FAOD, the actual reason why it was done that way was because no-one
> wanted to edit libxl_save_msgs_gen.pl to be able to cope with pointers,
> but "$WE don't want to do it properly" is not a reasonable justification.
This argument depends on
Re: [Xen-devel] [PATCH v5.1 7/8] os-posix: Provide new -runas : facility
Anthony PERARD writes ("Re: [PATCH v5.1 7/8] os-posix: Provide new -runas
: facility"):
> On Fri, Oct 20, 2017 at 02:38:21PM +0100, Ian Jackson wrote:
> > +static bool os_parse_runas_uid_gid(const char *optarg)
...
> > +errno = 0;
> > +lv = strtoul(optarg, , 0); /* can't qemu_strtoul, want *ep==':' */
>
> Should strtoul base be 10? If that matter.
If someone wants to write uids in hex then I don't see a reason to
stop them...
> > -if (!user_pwd) {
> > -fprintf(stderr, "User \"%s\" doesn't exist\n", optarg);
> > +if (!user_pwd && !os_parse_runas_uid_gid(optarg)) {
> > +fprintf(stderr,
> > +"User \"%s\" doesn't exist (and is not .)\n",
>
> The error message have not been update, I think it should be :
Oops.
> With the error message fix:
> Reviewed-by: Anthony PERARD <anthony.per...@citrix.com>
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] scripts: introduce a script for build test
Anthony PERARD writes ("Re: [PATCH v2] scripts: introduce a script for build
test"):
> That feels wrong. How do I run the same exact command at the default
> one, but with -j8 instead of -j4?
.../build-test sh -ec make -j4 distclean && ./configure && make -j4
But I think Anthony has a point. The clean should 1. be git-clean,
not make distclean 2. be run anyway.
> > +echo "Restoring original HEAD"
> > +git checkout $ORIG_BRANCH
>
> Also, what a developper should do when the build fail? She can't modify
> the current code, because changes are going to be losts. Maybe we could
> trap failures, restore original HEAD and point out which commit fails to
> build.
IWBNI it would at least print the branch to checkout. Tools like "git
bisect" record the information in .git and allow "git bisect reset".
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 00/33] osstest: FreeBSD host support
Roger Pau Monné writes ("Re: [PATCH v12 00/33] osstest: FreeBSD host support"):
> Sorry for the delay, had to cherry-pick some commits from the FreeBSD
> host install series in order for the examine one to work. I've pushed
> this to the following branch:
>
> git://xenbits.xen.org/people/royger/osstest.git examine
I have now rebased this onto my small queue (of patches to admin tools
which should not cause trouble) and pushed it to pretest.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] mg-hosts: Fix usage of showprops
Anthony PERARD writes ("[PATCH] mg-hosts: Fix usage of showprops"):
> ./mg-hosts showprops XXX description and implementation didn't match.
> Fix description.
Thanks, queued. (I edited the commit message a bit.)
Ian.
From 10ce43294150c6cfd5b00154636eb8cb945b7188 Mon Sep 17 00:00:00 2001
From: Anthony PERARD <anthony.per...@citrix.com>
Date: Tue, 24 Oct 2017 11:37:20 +0100
Subject: [OSSTEST PATCH] mg-hosts: Fix of showprops doc comment
./mg-hosts showprops description and implementation didn't match.
Fix description.
Signed-off-by: Anthony PERARD <anthony.per...@citrix.com>
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
---
mg-hosts | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mg-hosts b/mg-hosts
index a000f2d..d91a965 100755
--- a/mg-hosts
+++ b/mg-hosts
@@ -54,8 +54,8 @@
# old tasks might still mess about with the resources,
# interfering with whatever new tasks allocate them.
#
-# ./mg-hosts showprops [HOST...]
-# Prints the resource properties of all or specified hosts.
+# ./mg-hosts showprops [PROP...]
+# Prints all or specified resource properties of all hosts.
#
# ./mg-hosts setprops HOSTGLOB... [- PROP [OLD] NEW ...] -|-- PROP [OLD] NEW
# Updates resource properties of the specified hosts.
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] scripts: introduce a script for build test
Wei Liu writes ("[PATCH v2] scripts: introduce a script for build test"):
...
> +if git branch | grep -q '^\*.\+detached at'; then
You mean some rune involving git-symbolic-ref.
git-symbolic-ref -q HEAD exits with status 1 if HEAD is detached, 0 if
HEAD is a branch, or some other status in case of trouble.
But you could combine this test with your ORIG_BRANCH thing, and just
say
git-symbolic-ref HEAD
which exits 128 if HEAD is detached.
> +if ! git merge-base --is-ancestor $BASE $TIP; then
> +echo "$BASE is not an ancestor of $TIP, aborted"
> +exit 1
> +fi
I would remove this check. There is nothing wrong with asking "does
this branch build everywhere" even if it hasn't rebased yet.
The rest LGTM.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing
Wei Liu writes ("Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for
build testing"):
> On Mon, Oct 23, 2017 at 03:50:31PM +0100, Anthony PERARD wrote:
> > FIY, I do like to put script and other files in my checkouts, the git
> > clean will remove them.
>
> I changed that to make distclean this morning.
Urgh. This script depends on git, so please continue to use git to
check if the tree is clean.
The right answer would be to *check that the tree is clean* before
starting.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [xen-unstable test] 115037: regressions - FAIL
Julien Grall writes ("Re: [Xen-devel] [xen-unstable test] 115037: regressions -
FAIL"):
> Would it be possible of a platform specific bug? The last two flights
> are failing on merlot1.
The merlots are a highly unusual AMD machines which have NUMA nodes
with no memory and seem to sometimes have performance problems...
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10] libxl: annotate s to be nonnull in libxl__enum_from_string
Wei Liu writes ("Re: [Xen-devel] [PATCH for-4.10] libxl: annotate s to be
nonnull in libxl__enum_from_string"):
> On Mon, Oct 23, 2017 at 01:32:50PM +0100, Julien Grall wrote:
> > I would be ok with that. Wei do you have any opinion?
>
> Sure this is a simple enough patch. We should preferably turn all NN1 to
> NN(1), too.
That would be fine by me but I don't feel a need to hurry with that.
I can provide the patch to do that right now, or we can save doing
that for after 4.10.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing
Wei Liu writes ("Re: [PATCH for-4.10] scripts: add a script for build testing"):
> On Mon, Oct 23, 2017 at 02:24:40AM -0600, Jan Beulich wrote:
> > On 20.10.17 at 19:32, wrote:
> > > +git rebase $BASE $TIP -x "$CMD"
> >
> > Is this quoting on $CMD really going to work right no matter what
> > the variable actually expands to? I.e. don't you either want to use
> > "eval" or adjust script arguments such that you can use "$@" with
> > its special quoting rules?
Yes. Jan is completely right.
> What sort of use cases you have in mind that involve complex quoting and
> expansion?
There is really no excuse at all, in a script like this, for not using
`shift' to eat the main positional parameters, and then executing
"$@", faithfully reproducing the incoming parameters.
Of course there is a problem with getting this through git-rebase but
as I have just pointed out, git-rev-list and git-checkout are much
more suitable building blocks than git-rebase (which does a lot of
undesirable stuff that has to be suppressed, etc.)
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing
Wei Liu writes ("Re: [PATCH for-4.10] scripts: add a script for build testing"):
> On Mon, Oct 23, 2017 at 01:02:00PM +0100, Ian Jackson wrote:
> > In particular, if you:
> > * check that the tree is not dirty
> > * detach HEAD
>
> I think these two checks are good.
>
> > * reattach HEAD afterwards at least on success
>
> This is already the case for git-rebase on success.
No. git-rebase _rewrites_ HEAD.
Your script should just check out the intermediate commits. You
probably don't in fact want git-rebase. In particular, you don't want
to risk merge conflicts.
I have a script I use for dgit testing that looks like this:
#!/bin/bash
#
# run git fetch main
# and then this
set -e
set -o pipefail
revspec=main/${STTM_TESTED-tested}..main/pretest
echo "testing $revspec ..."
git-rev-list $revspec | nl -ba | tac | \
while read num rev; do
echo >&2 ""
echo >&2 "testing $num $rev"
git checkout $rev
${0%/*}/sometest-to-tested
done
FAOD,
Signed-off-by: Ian Jackson <ijack...@chiark.greenend.org.uk>
for inclusion of parts of this in the Xen build system.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing
Wei Liu writes ("Re: [PATCH for-4.10] scripts: add a script for build testing"):
> On Mon, Oct 23, 2017 at 02:24:40AM -0600, Jan Beulich wrote:
> > What is this startup delay intended for?
>
> To give user a chance to check the command -- git-rebase can be
> destructive after all.
I can't resist this bikeshed. This kind of thing is quite annoying.
If your command might be destructive, why not fix it so that it's not
destructive.
In particular, if you:
* check that the tree is not dirty
* detach HEAD
* reattach HEAD afterwards at least on success
then the risk of lossage is low and you can safely just go ahead.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 4/5] tools: libxendevicemodel: Provide xendevicemodel_add_to_physmap
Ross Lagerwall writes ("[PATCH v2 4/5] tools: libxendevicemodel: Provide
xendevicemodel_add_to_physmap"):
> Signed-off-by: Ross Lagerwall <ross.lagerw...@citrix.com>
Assuming the hypervisor parts go in:
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5.1 8/8] configure: do_compiler: Dump some extra info under bash [and 1 more messages]
Ian Jackson writes ("[PATCH v5.1 8/8] configure: do_compiler: Dump some extra
info under bash"):
> This makes it much easier to find a particular thing in config.log.
>
> The information may be lacking in other shells, resulting in harmless
> empty output. (This is why we don't use the proper ${FUNCNAME[*]}
> array syntax - other shells will choke on that.)
>
> The extra output is only printed if configure is run with bash. The
> something), it is necessary to say bash ./configure to get the extra
> debug info in the log.
Kent Spillner points out that this last sentence is garbled. The
paragraph should read:
The extra output is only printed if configure is run with bash. On
systems where /bin/sh is not bash, it is necessary to say bash
./configure to get the extra debug info in the log.
I have updated it in my branch.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] linux-arm-xen branch, commit access, etc.
Stefano Stabellini writes ("Re: [Xen-devel] linux-arm-xen branch, commit
access, etc."):
> On Fri, 20 Oct 2017, Konrad Rzeszutek Wilk wrote:
> > 3. Use upstream released kernels. Follow them when they are released.
>
> I agree with Konrad. The reason why that branch is there is that
> initially we needed a couple of patches to run Linux on Exynos5 boards
> (Arndale). Today, vanilla releases should work. For example, 4.13 has
> everything we need as far as I can tell. I think it is time to remove
> the special branch.
So vanilla kernels are going to work well on our new ARM64 boxes, eg
ThunderX, and whatever we come up with for new ARM32 testing too ?
In.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10] docs: update coverage.markdown
Wei Liu writes ("[PATCH for-4.10] docs: update coverage.markdown"):
> The coverage support in hypervisor is redone. Update the document.
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 00/33] osstest: FreeBSD host support
We have decided: We will push the anoint and examine parts of this series to osstest pretest. (You're going to give me a suitable branch on Monday.) This should work because we have anointed FreeBSD builds already. If this works (passes pretest) we will then run a special invocation of the real examination flight (which normally only runs once a month) to collect the new host properties. All being well we will then run mg-branch-setup and try to run the full series (including the new freebsd tests) with --real, and then, if that is good, push it to pretest (where the self-test should then be a formality). Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 11/16] ts-debian-fixup: use correct resume device
Wei Liu writes ("Re: [OSSTEST PATCH 11/16] ts-debian-fixup: use correct resume
device"):
> On Fri, Oct 20, 2017 at 02:42:53PM +0100, Ian Jackson wrote:
> > Wei Liu writes ("Re: [OSSTEST PATCH 11/16] ts-debian-fixup: use correct
> > resume device"):
> > I'm not sure what you mean here. We are using the config template
> > provided by xen-tools. The reuse of the host's initrd is suggested by
> > the xen-create-image documentation.
>
> TBH I don't know what to write in a report. What is the expected
> behaviour?
How about this ? Adjust to taste, correcting any
lies/misunderstandings, maybe adding references, etc.
Steps to reproduce:
Follow instructions in xen-create-image, including suggestion to
re-use host's initrd, to make a guest running stretch. Try to boot
it.
Expected behaviour:
Guest boots promptly.
Observed behaviour:
Guest pauses for a long time during boot waiting for the "resume
device" to become available.
Analysis:
This is because the initrd has the host's swap device configured.
With stretch, and systemd, the delay is longer; earler Debian
releases had a similar issue but the shorter timeout meant it was a
less serious problem.
Suggested remedy:
It is unfortunate that the initrd has the resume device baked into
it. Normally this kind of thing (eg root=) comes via the command
line. So maybe the bug is in the initrd generation.
Alternatively, maybe xen-create-image should write a setting "extra"
into the domain config file "resume=".
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 31/33] ts-examine-hostprops-save: introduce a script to save properties
Roger Pau Monne writes ("[PATCH v12 31/33] ts-examine-hostprops-save: introduce
a script to save properties"):
> This script turns the properties stored in the runvars using the
> format hostprop/$ident/$prop=$val into host properties stored in the
> database.
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
Although,
> +foreach my $k (sort keys %r) {
> +next unless $k =~ m/^hostprop\/([^\/]*)\/([^\/]*)$/;
> +my $ho = selecthost($1);
> +my $prop = $2;
> +
> +logm("recording for $ho->{Name} $prop=$r{$k}");
> +
> +# NB: in order to aid debug only attempt to save the host props
> +# on flights with real blessing, for the rest just do a dry run.
> +if ($blessing eq "real") {
> +set_host_property($ho, $prop, $r{$k});
> +} else {
> +logm("not saving host prop with blessing $blessing != real");
This will be annoyingly noisy in non-real flights with multiple
hostprops.
Better would be to move the $blessing eq "real" test to the outside of
the loop and have a variable to day whether to actually set things.
Then the log message about "not saving" could be printed once.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 33/33] sg-run-job: hook the memdisk test into examine
Roger Pau Monne writes ("[PATCH v12 33/33] sg-run-job: hook the memdisk test
into examine"):
> Hook the memdisk parameter detection and the saving of the host
> properties into the examine jobs.
>
> Signed-off-by: Roger Pau Monné <roger@citrix.com>
> ---
> Changes since v2:
> - Do not pass a host ident to ts-examine-hostprops-save.
> - Use .- for ts-memdisk-try-append so that the rest of the job will
> run even if this step fails.
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 29/33] ts-freebsd-host-install: add arguments to test memdisk append options
Roger Pau Monne writes ("[PATCH v12 29/33] ts-freebsd-host-install: add
arguments to test memdisk append options"):
> This is needed in order to figure out which memdisk options should be
> used to boot the images on each specific box.
>
> Note that when passed the --recordappend argument upon success the
> script stores the tentative host property in the runvars.
Last time, I said:
Roger Pau Monne writes ("[PATCH OSSTEST v2 07/11] ts-freebsd-host-install:
add arguments to test memdisk append options"):
> This is needed in order to figure out which memdisk options should be
> used to boot the images on each specific box.
>
> Note that upon success the script stores the tentative host property
> in the runvars.
...
> +} elsif ($ARGV[0] eq "--recordappend") {
> +$record_append = 1;
...
> +if ($bootonly) {
> +hostprop_putative_record($ho, "MemdiskAppend", $memdisk_append)
> +if $record_append;
> +exit 0;
This is surely wrong.
The same code seems to be here, unchanged:
> +if ($bootonly) {
> +hostprop_putative_record($ho, "MemdiskAppend", $memdisk_append)
> +if $record_append;
> +exit 0;
> +}
What I mean is that you only do the work for $record_append if
$bootonly is also set. If --record-append is meaningful only with
--test-boot then you should die if it's specified without --test-boot.
Also, I have just noticed that the option names ought to be
--record-append --test-boot --memdisk-append
(ie with the hyphens that are conventional in multi-word long option
names).
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 26/33] HostDB: introduce set_property
Roger Pau Monne writes ("[PATCH v12 26/33] HostDB: introduce set_property"):
> And provide a helper in TestSupport to use it. This allows osstest to
> set host properties from test script themselves (instead of using
> the mg-hosts clu).
>
> Note that the setting of host properties is limited to flights with
> intended blessing real, and it will fail for any other blessing.
...
> +die "Attempting to modify host props with blessing $blessing != real"
> +if $blessing ne "real";
Thanks. You should write "intended blessing" in the commit message
and error message, since there is also a current blessing etc.
With that changed,
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] linux-arm-xen branch, commit access, etc.
Currently we are running our ARM tests in osstest off a branch in Stefano's personal Linux tree. This is a bit unsatisfactory. We would like to switch to a branch that Julien can push to too, and that is in a more official place. There are two options: 1. Create an ARM-specific Xen tree. Currently we don't have many ARM-specific trees but I don't see a problem with this. So we could create /arm/linux.git on xenbits. 2. Use a branch name within /linux-pvops.git, the main Linux tree on xenbits. This would mean making Julien a committer. What do people think would be best ? Also, separately, surely Julien should be listed in MAINTAINERS for the Xen ARM stuff in upstream Linux ? He doesn't seem to be at least in the version I have lying about here... Thanks, Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 11/16] ts-debian-fixup: use correct resume device
Wei Liu writes ("Re: [OSSTEST PATCH 11/16] ts-debian-fixup: use correct resume
device"):
> On Fri, Oct 20, 2017 at 02:20:29PM +0100, Ian Jackson wrote:
> > I don't think the guest booting "eventually" after some timeout is
> > correct.
>
> The OS is honoring whatever is written in the resume file. That's the
> correct behaviour to me.
>
> Booting after trying is better than not booting at all. That parameter
> isn't vital to a functional system. This has always been the case since
> forever. It is only the "timeout" in the past is shorter so osstest
> never noticed it.
>
> What is not correct is the content of the file. We can't blame the guest
> OS for that.
I agree with all of this. But, the overall behaviour is not correct.
Even though all of the things that osstest specified to
xen-create-image were correct. Therefore there is a bug.
> > > There is no option in xen-create-image to manipulate extra=. The only
> > > viable solution is to provide a new template. That seems overkill and
> > > would require more code.
> >
> > What you're saying is that this infelicity in xen-tools is not
> > particularly easy to fix. That doesn't mean it shouldn't be reported
> > as a bug.
>
> There is nothing to fix. System administrator should provide a proper
> template -- that's what xen-tools developers most likely to say.
I'm not sure what you mean here. We are using the config template
provided by xen-tools. The reuse of the host's initrd is suggested by
the xen-create-image documentation.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5.1 1/8] xen: link against xentoolcore
From: Anthony PERARD <anthony.per...@citrix.com>
Xen libraries in 4.10 include a new xentoolcore library. This
contains the xentoolcore_restrict_all function which we are about to
want to use.
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
---
v5: More truthful commit message.
---
configure | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index fd7e3a5..6f691df 100755
--- a/configure
+++ b/configure
@@ -2072,7 +2072,7 @@ if test "$xen" != "no" ; then
$($pkg_config --modversion xencontrol | sed 's/\./ /g') )"
xen=yes
xen_pc="xencontrol xenstore xenguest xenforeignmemory xengnttab"
-xen_pc="$xen_pc xenevtchn xendevicemodel"
+xen_pc="$xen_pc xenevtchn xendevicemodel xentoolcore"
QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags $xen_pc)"
libs_softmmu="$($pkg_config --libs $xen_pc) $libs_softmmu"
LDFLAGS="$($pkg_config --libs $xen_pc) $LDFLAGS"
@@ -2104,18 +2104,20 @@ EOF
cat > $TMPC <
+#include
int main(void) {
xenforeignmemory_handle *xfmem;
xfmem = xenforeignmemory_open(0, 0);
xenforeignmemory_map2(xfmem, 0, 0, 0, 0, 0, 0, 0);
+ xentoolcore_restrict_all(0);
return 0;
}
EOF
-compile_prog "" "$xen_libs -lxendevicemodel $xen_stable_libs"
+compile_prog "" "$xen_libs -lxendevicemodel $xen_stable_libs
-lxentoolcore"
then
- xen_stable_libs="-lxendevicemodel $xen_stable_libs"
+ xen_stable_libs="-lxendevicemodel $xen_stable_libs -lxentoolcore"
xen_ctrl_version=41000
xen=yes
elif
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5.1 3/8] xen: defer call to xen_restrict until just before os_setup_post
We need to restrict *all* the control fds that qemu opens. Looking in
/proc/PID/fd shows there are many; their allocation seems scattered
throughout Xen support code in qemu.
We must postpone the restrict call until roughly the same time as qemu
changes its uid, chroots (if applicable), and so on.
There doesn't seem to be an appropriate hook already. The RunState
change hook fires at different times depending on exactly what mode
qemu is operating in.
And it appears that no-one but the Xen code wants a hook at this phase
of execution. So, introduce a bare call to a new function
xen_setup_post, just before os_setup_post. Also provide the
appropriate stub for when Xen compilation is disabled.
We do the restriction before rather than after os_setup_post, because
xen_restrict may need to open /dev/null, and os_setup_post might have
called chroot.
Currently this does not work with migration, because when running as
the Xen device model qemu needs to signal to the toolstack that it is
ready. It currently does this using xenstore, and for incoming
migration (but not for ordinary startup) that happens after
os_setup_post.
It is correct that this happens late: we want the incoming migration
stream to be processed by a restricted qemu. The fix for this will be
to do the startup notification a different way, without using
xenstore. (QMP is probably a reasonable choice.)
So for now this restriction feature cannot be used in conjunction with
migration. (Note that this is not a regression in this patch, because
previously the -xen-restrict-domid call was, in fact, simply
ineffective!) We will revisit this in the Xen 4.11 release cycle.
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
Reviewed-by: Anthony PERARD <anthony.per...@citrix.com>
---
v5: Discuss problems with migration startup notification
in the commit message.
v3: Do xen_setup_post just before, not just after, os_setup_post,
to improve interaction with chroot. Thanks to Ross Lagerwall.
---
hw/i386/xen/xen-hvm.c | 8
hw/xen/xen-common.c | 13 +
include/sysemu/sysemu.h | 2 ++
stubs/xen-hvm.c | 5 +
vl.c| 1 +
5 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index d9ccd5d..7b60ec6 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1254,14 +1254,6 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion
**ram_memory)
goto err;
}
-if (xen_domid_restrict) {
-rc = xen_restrict(xen_domid);
-if (rc < 0) {
-error_report("failed to restrict: error %d", errno);
-goto err;
-}
-}
-
xen_create_ioreq_server(xen_domid, >ioservid);
state->exit.notify = xen_exit_notifier;
diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
index 632a938..4056420 100644
--- a/hw/xen/xen-common.c
+++ b/hw/xen/xen-common.c
@@ -117,6 +117,19 @@ static void xen_change_state_handler(void *opaque, int
running,
}
}
+void xen_setup_post(void)
+{
+int rc;
+
+if (xen_domid_restrict) {
+rc = xen_restrict(xen_domid);
+if (rc < 0) {
+perror("xen: failed to restrict");
+exit(1);
+}
+}
+}
+
static int xen_init(MachineState *ms)
{
xen_xc = xc_interface_open(0, 0, 0);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index b213696..b064a55 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -93,6 +93,8 @@ void qemu_remove_machine_init_done_notifier(Notifier *notify);
void qemu_announce_self(void);
+void xen_setup_post(void);
+
extern int autostart;
typedef enum {
diff --git a/stubs/xen-hvm.c b/stubs/xen-hvm.c
index 3ca6c51..9701feb 100644
--- a/stubs/xen-hvm.c
+++ b/stubs/xen-hvm.c
@@ -13,6 +13,7 @@
#include "hw/xen/xen.h"
#include "exec/memory.h"
#include "qmp-commands.h"
+#include "sysemu/sysemu.h"
int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
{
@@ -61,3 +62,7 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion
**ram_memory)
void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
{
}
+
+void xen_setup_post(void)
+{
+}
diff --git a/vl.c b/vl.c
index fb1f05b..ca06553 100644
--- a/vl.c
+++ b/vl.c
@@ -4792,6 +4792,7 @@ int main(int argc, char **argv, char **envp)
vm_start();
}
+xen_setup_post();
os_setup_post();
main_loop();
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5.1 5/8] xen: move xc_interface compatibility fallback further up the file
We are going to want to use the dummy xendevicemodel_handle type in
new stub functions in the CONFIG_XEN_CTRL_INTERFACE_VERSION < 41000
section. So we need to provide that definition, or (as applicable)
include the appropriate header, earlier in the file.
(Ideally the newer compatibility layers would be at the bottom of the
file, so that they can naturally benefit from the compatibility layers
for earlier version. But that's rather too much for this series.)
No functional change.
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
Acked-by: Anthony PERARD <anthony.per...@citrix.com>
---
v2: New patch in v2 of the series
---
include/hw/xen/xen_common.h | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 3f44a63..8efdb8a 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -78,6 +78,17 @@ static inline void *xenforeignmemory_map(xc_interface *h,
uint32_t dom,
extern xenforeignmemory_handle *xen_fmem;
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40900
+
+typedef xc_interface xendevicemodel_handle;
+
+#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40900 */
+
+#undef XC_WANT_COMPAT_DEVICEMODEL_API
+#include
+
+#endif
+
#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 41000
#define XEN_COMPAT_PHYSMAP
@@ -105,8 +116,6 @@ static inline int xentoolcore_restrict_all(domid_t domid)
#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40900
-typedef xc_interface xendevicemodel_handle;
-
static inline xendevicemodel_handle *xendevicemodel_open(
struct xentoollog_logger *logger, unsigned int open_flags)
{
@@ -228,11 +237,6 @@ static inline int xendevicemodel_set_mem_type(
return xc_hvm_set_mem_type(dmod, domid, mem_type, first_pfn, nr);
}
-#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40900 */
-
-#undef XC_WANT_COMPAT_DEVICEMODEL_API
-#include
-
#endif
extern xendevicemodel_handle *xen_dmod;
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5.1 4/8] xen: destroy_hvm_domain: Move reason into a variable
We are going to want to reuse this.
No functional change.
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
Reviewed-by: Anthony PERARD <anthony.per...@citrix.com>
---
hw/i386/xen/xen-hvm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 7b60ec6..83420cd 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1387,12 +1387,13 @@ void destroy_hvm_domain(bool reboot)
xc_interface *xc_handle;
int sts;
+unsigned int reason = reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff;
+
xc_handle = xc_interface_open(0, 0, 0);
if (xc_handle == NULL) {
fprintf(stderr, "Cannot acquire xenctrl handle\n");
} else {
-sts = xc_domain_shutdown(xc_handle, xen_domid,
- reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff);
+sts = xc_domain_shutdown(xc_handle, xen_domid, reason);
if (sts != 0) {
fprintf(stderr, "xc_domain_shutdown failed to issue %s, "
"sts %d, %s\n", reboot ? "reboot" : "poweroff",
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5.1 8/8] configure: do_compiler: Dump some extra info under bash
This makes it much easier to find a particular thing in config.log.
The information may be lacking in other shells, resulting in harmless
empty output. (This is why we don't use the proper ${FUNCNAME[*]}
array syntax - other shells will choke on that.)
The extra output is only printed if configure is run with bash. The
something), it is necessary to say bash ./configure to get the extra
debug info in the log.
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
---
v4: No longer tag this patch RFC.
---
configure | 4
1 file changed, 4 insertions(+)
diff --git a/configure b/configure
index 6f691df..21a2b15 100755
--- a/configure
+++ b/configure
@@ -60,6 +60,10 @@ do_compiler() {
# is compiler binary to execute.
local compiler="$1"
shift
+echo >>config.log "
+funcs: ${FUNCNAME}
+lines: ${BASH_LINENO}
+files: ${BASH_SOURCE}"
echo $compiler "$@" >> config.log
$compiler "$@" >> config.log 2>&1 || return $?
# Test passed. If this is an --enable-werror build, rerun
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5.1 2/8] xen: restrict: use xentoolcore_restrict_all
And insist that it works.
Drop individual use of xendevicemodel_restrict and
xenforeignmemory_restrict. These are not actually effective in this
version of qemu, because qemu has a large number of fds open onto
various Xen control devices.
The restriction arrangements are still not right, because the
restriction needs to be done very late - after qemu has opened all of
its control fds.
xentoolcore_restrict_all and xentoolcore.h are available in Xen 4.10
and later, only. Provide a compatibility stub. And drop the
compatibility stubs for the old functions.
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
Reviewed-by: Anthony PERARD <anthony.per...@citrix.com>
---
v2: Modify the compatibility code, too.
Bump this patch ahead of "defer call to xen_restrict until running"
Retain call to xentoolcore_restrict_all
---
include/hw/xen/xen_common.h | 46 +++--
1 file changed, 11 insertions(+), 35 deletions(-)
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 86c7f26..3f44a63 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -91,6 +91,16 @@ static inline void
*xenforeignmemory_map2(xenforeignmemory_handle *h,
return xenforeignmemory_map(h, dom, prot, pages, arr, err);
}
+static inline int xentoolcore_restrict_all(domid_t domid)
+{
+errno = ENOTTY;
+return -1;
+}
+
+#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 41000 */
+
+#include
+
#endif
#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40900
@@ -218,20 +228,6 @@ static inline int xendevicemodel_set_mem_type(
return xc_hvm_set_mem_type(dmod, domid, mem_type, first_pfn, nr);
}
-static inline int xendevicemodel_restrict(
-xendevicemodel_handle *dmod, domid_t domid)
-{
-errno = ENOTTY;
-return -1;
-}
-
-static inline int xenforeignmemory_restrict(
-xenforeignmemory_handle *fmem, domid_t domid)
-{
-errno = ENOTTY;
-return -1;
-}
-
#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40900 */
#undef XC_WANT_COMPAT_DEVICEMODEL_API
@@ -290,28 +286,8 @@ static inline int xen_modified_memory(domid_t domid,
uint64_t first_pfn,
static inline int xen_restrict(domid_t domid)
{
int rc;
-
-/* Attempt to restrict devicemodel operations */
-rc = xendevicemodel_restrict(xen_dmod, domid);
+rc = xentoolcore_restrict_all(domid);
trace_xen_domid_restrict(rc ? errno : 0);
-
-if (rc < 0) {
-/*
- * If errno is ENOTTY then restriction is not implemented so
- * there's no point in trying to restrict other types of
- * operation, but it should not be treated as a failure.
- */
-if (errno == ENOTTY) {
-return 0;
-}
-
-return rc;
-}
-
-/* Restrict foreignmemory operations */
-rc = xenforeignmemory_restrict(xen_fmem, domid);
-trace_xen_domid_restrict(rc ? errno : 0);
-
return rc;
}
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5.1 6/8] xen: destroy_hvm_domain: Try xendevicemodel_shutdown
xc_interface_open etc. is not going to work if we have dropped
privilege, but xendevicemodel_shutdown will if everything is new
enough.
xendevicemodel_shutdown is only availabe in Xen 4.10 and later, so
provide a stub for earlier versions.
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
Reviewed-by: Anthony PERARD <anthony.per...@citrix.com>
---
v2: Add compatibility stub for Xen < 4.10.
Fix coding style.
---
hw/i386/xen/xen-hvm.c | 10 ++
include/hw/xen/xen_common.h | 7 +++
2 files changed, 17 insertions(+)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 83420cd..25b8b14 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1386,9 +1386,19 @@ void destroy_hvm_domain(bool reboot)
{
xc_interface *xc_handle;
int sts;
+int rc;
unsigned int reason = reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff;
+if (xen_dmod) {
+rc = xendevicemodel_shutdown(xen_dmod, xen_domid, reason);
+if (!rc) {
+return;
+}
+perror("xendevicemodel_shutdown failed");
+/* well, try the old thing then */
+}
+
xc_handle = xc_interface_open(0, 0, 0);
if (xc_handle == NULL) {
fprintf(stderr, "Cannot acquire xenctrl handle\n");
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 8efdb8a..1d6fb57 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -108,6 +108,13 @@ static inline int xentoolcore_restrict_all(domid_t domid)
return -1;
}
+static inline int xendevicemodel_shutdown(xendevicemodel_handle *dmod,
+ domid_t domid, unsigned int reason)
+{
+errno = ENOTTY;
+return -1;
+}
+
#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 41000 */
#include
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5.1 7/8] os-posix: Provide new -runas : facility
This allows the caller to specify a uid and gid to use, even if there
is no corresponding password entry. This will be useful in certain
Xen configurations.
We don't support just -runas because: (i) deprivileging without
calling setgroups would be ineffective (ii) given only a uid we don't
know what gid we ought to use (since uids may eppear in multiple
passwd file entries with different gids).
Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
---
v5: Use : rather than . to separate uid from gid
v4: Changed to reuse option -runas
v3: Error messages fixed. Thanks to Peter Maydell and Ross Lagerwall.
v2: Coding style fixes.
squash! os-posix: Provide new -runas . facility
---
os-posix.c | 64 +++--
qemu-options.hx | 3 ++-
2 files changed, 55 insertions(+), 12 deletions(-)
diff --git a/os-posix.c b/os-posix.c
index 92e9d85..f95b7bf 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -43,6 +43,8 @@
#endif
static struct passwd *user_pwd;
+static uid_t user_uid = (uid_t)-1;
+static gid_t user_gid = (gid_t)-1;
static const char *chroot_dir;
static int daemonize;
static int daemon_pipe;
@@ -128,6 +130,34 @@ void os_set_proc_name(const char *s)
#endif
}
+
+static bool os_parse_runas_uid_gid(const char *optarg)
+{
+unsigned long lv;
+char *ep;
+uid_t got_uid;
+gid_t got_gid;
+int rc;
+
+errno = 0;
+lv = strtoul(optarg, , 0); /* can't qemu_strtoul, want *ep==':' */
+got_uid = lv; /* overflow here is ID in C99 */
+if (errno || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) {
+return false;
+}
+
+lv = 0;
+rc = qemu_strtoul(ep + 1, 0, 0, );
+got_gid = lv; /* overflow here is ID in C99 */
+if (rc || got_gid != lv || got_gid == (gid_t)-1) {
+return false;
+}
+
+user_uid = got_uid;
+user_gid = got_gid;
+return true;
+}
+
/*
* Parse OS specific command line options.
* return 0 if option handled, -1 otherwise
@@ -145,8 +175,10 @@ void os_parse_cmd_args(int index, const char *optarg)
#endif
case QEMU_OPTION_runas:
user_pwd = getpwnam(optarg);
-if (!user_pwd) {
-fprintf(stderr, "User \"%s\" doesn't exist\n", optarg);
+if (!user_pwd && !os_parse_runas_uid_gid(optarg)) {
+fprintf(stderr,
+"User \"%s\" doesn't exist (and is not .)\n",
+optarg);
exit(1);
}
break;
@@ -166,18 +198,28 @@ void os_parse_cmd_args(int index, const char *optarg)
static void change_process_uid(void)
{
-if (user_pwd) {
-if (setgid(user_pwd->pw_gid) < 0) {
-fprintf(stderr, "Failed to setgid(%d)\n", user_pwd->pw_gid);
+if (user_pwd || user_uid != (uid_t)-1) {
+gid_t intended_gid = user_pwd ? user_pwd->pw_gid : user_gid;
+uid_t intended_uid = user_pwd ? user_pwd->pw_uid : user_uid;
+if (setgid(intended_gid) < 0) {
+fprintf(stderr, "Failed to setgid(%d)\n", intended_gid);
exit(1);
}
-if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) {
-fprintf(stderr, "Failed to initgroups(\"%s\", %d)\n",
-user_pwd->pw_name, user_pwd->pw_gid);
-exit(1);
+if (user_pwd) {
+if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) {
+fprintf(stderr, "Failed to initgroups(\"%s\", %d)\n",
+user_pwd->pw_name, user_pwd->pw_gid);
+exit(1);
+}
+} else {
+if (setgroups(1, _gid) < 0) {
+fprintf(stderr, "Failed to setgroups(1, [%d])",
+user_gid);
+exit(1);
+}
}
-if (setuid(user_pwd->pw_uid) < 0) {
-fprintf(stderr, "Failed to setuid(%d)\n", user_pwd->pw_uid);
+if (setuid(intended_uid) < 0) {
+fprintf(stderr, "Failed to setuid(%d)\n", intended_uid);
exit(1);
}
if (setuid(0) != -1) {
diff --git a/qemu-options.hx b/qemu-options.hx
index 9f6e2ad..f707197e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3958,7 +3958,8 @@ ETEXI
#ifndef _WIN32
DEF("runas", HAS_ARG, QEMU_OPTION_runas, \
-"-runas user change to user id user just before starting the VM\n",
+"-runas user change to user id user just before starting the VM\n" \
+"user can be numeric uid:gid instead\n",
QEMU_ARCH_ALL)
#endif
STEXI
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 0/8] xen: xen-domid-restrict improvements
Anthony PERARD writes ("Re: [PATCH v5 0/8] xen: xen-domid-restrict
improvements"):
> The patches in this v5 appear to be the same the one from the patch
> series v4.
Erk, so they are.
I'll post a v5.1 in reply to this email.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 11/16] ts-debian-fixup: use correct resume device
Wei Liu writes ("Re: [OSSTEST PATCH 11/16] ts-debian-fixup: use correct resume
device"):
> I wouldn't call this behaviour a bug. Most people won't notice it
> because the guest will boot eventually. It is not like this will cause
> the guest to crash.
I don't think the guest booting "eventually" after some timeout is
correct.
> There is no option in xen-create-image to manipulate extra=. The only
> viable solution is to provide a new template. That seems overkill and
> would require more code.
What you're saying is that this infelicity in xen-tools is not
particularly easy to fix. That doesn't mean it shouldn't be reported
as a bug.
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 10/16] ts-debian-fixup: remove extra= before appending our own
Wei Liu writes ("Re: [OSSTEST PATCH 10/16] ts-debian-fixup: remove extra=
before appending our own"):
> On Fri, Oct 20, 2017 at 12:03:24PM +0100, Ian Jackson wrote:
> > Wei Liu writes ("[OSSTEST PATCH 10/16] ts-debian-fixup: remove extra=
> > before appending our own"):
> > > The original extra= was not removed, so there were two extra= in the
> > > resulting config file.
> >
> > What is the original extra= ? Why should we not combine them ?
>
> The original extra= is generated by xen-create-image. It has the content
> "elevator=noop". It doesn't seem too useful to me. But I'm fine with
> combination them.
I think they should be combined. And that "elevator=noop" doesn't
sound unreasonable.
> > > It wasn't a problem for xl because the second extra= took precedence.
> > > However libvirt tests would only pick up the first extra= -- they
> > > worked by chance.
> >
> > That's odd. Is this to do with the xl -> libxl config converter ?
> > It seems to me that that converter should interpret xl config files
> > the same way xl does. (Also xm did the same: last setting wins.)
>
> Yes, the converter only picks up the first.
This is a bug, then. Where should we file it ?
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 11/16] ts-debian-fixup: use correct resume device
Wei Liu writes ("Re: [OSSTEST PATCH 11/16] ts-debian-fixup: use correct resume
device"):
> On Fri, Oct 20, 2017 at 12:05:55PM +0100, Ian Jackson wrote:
> > Wei Liu writes ("[OSSTEST PATCH 11/16] ts-debian-fixup: use correct resume
> > device"):
> > > See code comment for explanation.
> > ...
> > > +# There might be stale entries in /etc/initramfs-tools/conf.d/resume
> > > +# which get stored in the initramfs. That introduces delay in guest
> > > booting
> > > +# which might cause tests to fail.
> >
> > Why might there be such stale entries ?
>
> The ramdisk is taken from the host, which contains that file. The resume
> device is going to point to the swap partition in the host.
Ah. Hrm. I guess this was always a bit of a bodge.
> > > +# Override that in kernel command line with the correct swap
> > > partition.
> > > +$cfg =~ m/'phy:.+-swap,(xvda\d+),.*'/;
> > > +$extra .= " resume=/dev/$1";
> > > +logm("change resume device to $1");
> >
> > Alternatively, if indeed this is necessary and not due to bugs,
> > perhaps it should be done by xen-tools ?
>
> No, see the first paragraph. xen-tools can't be expected to modify the
> ramdisk.
Maybe it should set the appropriate "extra" though. IIRC xen-tools
does implement this sort-of-trick of reusing the hosts initrd. So why
does this bug not happen with other users of xen-tools ?
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 16/16] ts-guests-nbd-mirror: make it work with stretch
Wei Liu writes ("[OSSTEST PATCH 16/16] ts-guests-nbd-mirror: make it work with
stretch"):
> On the server side, only add oldstyle= and port= on jessie. Stretch
> doesn't support or need those anymore.
See my earlier comments about old vs new Debian suite names.
> Prune check for older versions of Debian.
Why ? Certainly I can see no justification for cutting off squeeze.
Perhaps someone is still trying to test with squeeze. Also, doing
this in the same patch is quite confusing.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 11/16] ts-debian-fixup: use correct resume device
Wei Liu writes ("[OSSTEST PATCH 11/16] ts-debian-fixup: use correct resume
device"):
> See code comment for explanation.
...
> +# There might be stale entries in /etc/initramfs-tools/conf.d/resume
> +# which get stored in the initramfs. That introduces delay in guest
> booting
> +# which might cause tests to fail.
Why might there be such stale entries ?
> +# This is particularly prominent in systemd enabled releases when it
> tries
> +# to scan for the inexistent device(s) for a long time.
Why is this not a bug in systemd ? If there are workarounds in
osstest for Debian, I usually want a bug in the BTS, and a suite limit
on the workaround so that we poke Debian again in 2 years time...
> +# Override that in kernel command line with the correct swap partition.
> +$cfg =~ m/'phy:.+-swap,(xvda\d+),.*'/;
> +$extra .= " resume=/dev/$1";
> +logm("change resume device to $1");
Alternatively, if indeed this is necessary and not due to bugs,
perhaps it should be done by xen-tools ?
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 10/16] ts-debian-fixup: remove extra= before appending our own
Wei Liu writes ("[OSSTEST PATCH 10/16] ts-debian-fixup: remove extra= before
appending our own"):
> The original extra= was not removed, so there were two extra= in the
> resulting config file.
What is the original extra= ? Why should we not combine them ?
> It wasn't a problem for xl because the second extra= took precedence.
> However libvirt tests would only pick up the first extra= -- they
> worked by chance.
That's odd. Is this to do with the xl -> libxl config converter ?
It seems to me that that converter should interpret xl config files
the same way xl does. (Also xm did the same: last setting wins.)
>
> +
> +$cfg =~ s/^extra.*//mg;
> $cfg .= "\nextra='$extra'\n";
Also, accidental whitespace change.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 13/16] ts-debian-hvm-install: disable new nic naming scheme
Wei Liu writes ("[OSSTEST PATCH 13/16] ts-debian-hvm-install: disable new nic
naming scheme"):
> This is required to fix nested hvm test. The L1 host is installed by
> this script. We want the L1 host to not use the new nic naming scheme.
The principle is fine.
> +# Do not use "Predictable Network Interface Names" -- this can break
> +# nested HVM tests.
> +#
> https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
> +#
> +# See also
> +# https://www.debian.org/releases/stretch/example-preseed.txt
> +my $netifnames = "";
> +$netifnames = "\nd-i debian-installer/add-kernel-opts string
> net.ifnames=0\n"
> +if $ho->{Suite} =~ m/stretch/;
Please use a <{Suite}
d-i debian... etc. etc.
END
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 09/16] ts-host-install: don't use the new nic naming scheme
Wei Liu writes ("[OSSTEST PATCH 09/16] ts-host-install: don't use the new nic
naming scheme"):
> Signed-off-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 07/16] Debian.pm: use sysvinit-core on stretch
Wei Liu writes ("[OSSTEST PATCH 07/16] Debian.pm: use sysvinit-core on
stretch"):
...
> diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> index 845027a..24bc260 100644
> --- a/Osstest/Debian.pm
> +++ b/Osstest/Debian.pm
> @@ -827,7 +827,7 @@ sub preseed_base ($$$;@) {
>
> # Systemd doesn't honor osstest-confirm-booted service, which
> # breaks ts-leak-check. Fall back to SysV init for now.
> -if ( $suite =~ /jessie/ ) {
> +if ( $suite =~ /jessie|stretch/ ) {
> preseed_hook_command($ho, 'late_command', $sfx, < in-target apt-get install -y sysvinit-core
I think we should probably recognise that this is not going to change,
and switch this from a workaround to a policy decision. Ie,
if ... !~ m/squeeze/
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 08/16] ts-leak-check: suppress systemd-shim, which leaks in stretch
Wei Liu writes ("[OSSTEST PATCH 08/16] ts-leak-check: suppress systemd-shim,
which leaks in stretch"):
> Signed-off-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 05/16] mg-debian-installer-update-all: put quotes around arguments
Wei Liu writes ("[OSSTEST PATCH 05/16] mg-debian-installer-update-all: put
quotes around arguments"):
> Signed-off-by: Wei Liu
...
> suite=$1
> arch=$2
> -packages="$3"
> +packages=$3
Not sure why this needless style change, but if you did it
deliberately I don't really mind...
> site=http://ftp.debian.org/debian/
> sbase=$site/dists/$suite
> diff --git a/mg-debian-installer-update-all b/mg-debian-installer-update-all
> index d88ebf5..d590b2b 100755
> --- a/mg-debian-installer-update-all
> +++ b/mg-debian-installer-update-all
> @@ -31,5 +31,5 @@ fws=`getconfig DebianNonfreeFirmware`
> arches="arm64 armhf amd64 i386"
>
> for arch in $arches ; do
> -./mg-debian-installer-update $suite $arch $fws
> +./mg-debian-installer-update "$suite" "$arch" "$fws"
> done
This hunk LGTM. Although the "" around $suite and $arch are
unnecessary I don't really mind them.
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 01/16] XXX add a stretch config based on production-config
Wei Liu writes ("[OSSTEST PATCH 01/16] XXX add a stretch config based on
production-config"):
> diff -ub production-config production-config-stretch
The changes LGTM but obviously this ought to go straight into
`production-config'.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [OSSTEST PATCH 02/16] gitignore: ignore vim swap file
Wei Liu writes ("[OSSTEST PATCH 02/16] gitignore: ignore vim swap file"):
> Signed-off-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Ian Jackson <ian.jack...@eu.citrix.com>
Although, you may find your life improved by putting this in your
~/.config/git/ignore.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 04/16] ts-xen-build-prep: install packages for stretch
Wei Liu writes ("[OSSTEST PATCH 04/16] ts-xen-build-prep: install packages for
stretch"):
> Stubdom build needs texinfo.
Same comment as my previous patch. You should only mention old
release names in these kind of tests, unless you know that the requirement
is specific to only stretch and not future releases.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
