Commit Graph
1881 Commits
Author SHA1 Message Date
Ian Jackson 42f4d98f21 libxl: Fix bootloader-related virtual memory leak on pv build failure
The bootloader may call libxl__file_reference_map(), which mmap's the
pv_kernel and pv_ramdisk into process memory.  This was only unmapped,
however, on the success path of libxl__build_pv().  If there were a
failure anywhere between libxl_bootloader.c:parse_bootloader_result()
and the end of libxl__build_pv(), the calls to
libxl__file_reference_unmap() would be skipped, leaking the mapped
virtual memory.

Ideally this would be fixed by adding the unmap calls to the
destruction path for libxl__domain_build_state.  Unfortunately the
lifetime of the libxl__domain_build_state is opaque, and it doesn't
have a proper destruction path.  But, the only thing in it that isn't
from the gc are these bootloader references, and they are only ever
set for one libxl__domain_build_state, the one which is
libxl__domain_create_state.build_state.

So we can clean up in the exit path from libxl__domain_create_*, which
always comes through domcreate_complete.

Remove the now-redundant unmaps in libxl__build_pv's success path.

This is XSA-160.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Tested-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit 59543a7cc2)
2015-12-08 15:31:33 +00:00
Ian Jackson 423d2cd814 libxl: adjust PoD target by memory fudge, too
PoD guests need to balloon at least as far as required by PoD, or risk
crashing.  Currently they don't necessarily know what the right value
is, because our memory accounting is (at the very least) confusing.

Apply the memory limit fudge factor to the in-hypervisor PoD memory
target, too.  This will increase the size of the guest's PoD cache by
the fudge factor LIBXL_MAXMEM_CONSTANT (currently 1Mby).  This ensures
that even with a slightly-off balloon driver, the guest will be
stable even under memory pressure.

There are two call sites of xc_domain_set_pod_target that need fixing:

The one in libxl_set_memory_target is straightforward.

The one in xc_hvm_build_x86.c:setup_guest is more awkward.  Simply
setting the PoD target differently does not work because the various
amounts of memory during domain construction no longer match up.
Instead, we adjust the guest memory target in xenstore (but only for
PoD guests).

This introduces a 1Mby discrepancy between the balloon target of a PoD
guest at boot, and the target set by an apparently-equivalent `xl
mem-set' (or similar) later.  This approach is low-risk for a security
fix but we need to fix this up properly in xen.git#staging and
probably also in stable trees.

This is XSA-153.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
(cherry picked from commit 56fb5fd623)
2015-10-29 15:12:04 +00:00
Ian Jackson 47db4b088d Revert "libxl: use correct command line for arm guests."
This reverts commit 9befcd335c.

This commit breaks the build:
 libxl_arm.c: In function 'libxl__arch_domain_init_hw_description':
 libxl_arm.c:591:76: error: 'state' undeclared (first use in this function)
 libxl_arm.c:591:76: note: each undeclared identifier is reported only once fo\
r each function it appears in
 make[3]: *** [libxl_arm.o] Error 1

"state" was introduced in a7511905 "xen: Extend DOMCTL createdomain to
support arch configuration".

On Julien's recommendation: a7511905 ought not to be backported, so
revert this and wait for Ian Campbell to get back.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Julien Grall <julien.grall@citrix.com>
CC: Ian Campbell <Ian.Campbell@eu.citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
2015-10-26 11:25:07 +00:00
Ian Campbell 9befcd335c libxl: use correct command line for arm guests.
We need to use libxl__domain_build_state.pv_cmdline in order to pickup
the correct args when using pygrub. libxl_domain_build_info.cmdline is
any args statically configured by the user.

This is consistent with the call to xc_domain_allocate, which takes
the cmdline too (in that case for x86/PV usage).

state->pv_cmdline is also set for non-pygrub guests, since
libxl__bootloader_run propagates info->cmdline if no bootloader is
configured.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
(cherry picked from commit 201eac8383)
2015-10-21 17:08:56 +01:00
Ian Campbell 5461ad29be tools: libxl: allow permissive qemu-upstream pci passthrough.
Since XSA-131 qemu-xen now restricts access to PCI cfg by default. In
order to allow local configuration of the existing libxl_device_pci
"permissive" flag needs to be plumbed through via the new QMP property
added by the XSA-131 patches.

Versions of QEMU prior to XSA-131 did not support this permissive
property, so we only pass it if it is true. Older versions only
supported permissive mode.

qemu-xen-traditional already supports the permissive mode setting via
xenstore.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
(cherry picked from commit 015a373351)
2015-10-20 17:57:04 +01:00
Stefano Stabellini 0619913669 libxl: handle read-only drives with qemu-xen
The current libxl code doesn't deal with read-only drives at all.

Upstream QEMU and qemu-xen only support read-only cdrom drives: make
sure to specify "readonly=on" for cdrom drives and return error in case
the user requested a non-cdrom read-only drive.

This is XSA-142, discovered by Lin Liu
(https://bugzilla.redhat.com/show_bug.cgi?id=1257893).

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Backport to Xen 4.5 and earlier, apropos of report and review from
Michael Young.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
2015-09-22 16:56:18 +01:00
Anthony PERARD bbbd29a25d libxl: Increase device model startup timeout to 1min.
On a busy host, QEMU may take more than 10s to load and start.

This is likely due to a bug in Linux where the I/O subsystem sometime
produce high latency under load and result in QEMU taking a long time to
load every single dynamic libraries.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 9acfbe14d7)

Conflicts:
	tools/libxl/libxl_internal.h
2015-09-18 16:29:08 +01:00
Wei Liu ffb4e6387f xl: correct handling of extra_config in main_cpupoolcreate
Don't dereference extra_config if it's NULL. Don't leak extra_config in
the end.

Also fixed a typo in error string while I was there.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 705c9e1242)
2015-09-11 11:53:14 +01:00
Ian Jackson 7f7642f778 libxl: poll: Avoid fd deregistration race POLLNVAL crash
It can happen that an fd is deregistered, and closed, and then a new
fd opened, and reregistered, all while another thread is in poll().

If this happens poll might report POLLNVAL, but the event loop would
think that the fd was supposed to have been valid, and then fail an
assertion:
  libxl_event.c:1183: afterpoll_check_fd: Assertion `poller->fds_changed || !(fds[slot].revents & 0x020)' failed.

We can't simply ignore POLLNVAL because if we have bugs which cause
messed-up fds, it is a serious problem which we really need to detect.

Instead, add extra tracking to spot when this possibility arises, and
abort on POLLNVAL if we are sure that it is unexpected.

Reported-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
(cherry picked from commit 681ce16816)
2015-08-12 12:44:30 +01:00
Ian Jackson 9f6f513eec libxl: poll: Use poller_get and poller_put for poller_app
This makes the code more regular.  We are going to want to do some
more work in poller_get and poller_put, which work also wants to be
done for poller_app.

Two very minor functional changes:

 * We call malloc an extra time since poller_app is now a pointer

 * ERROR_FAIL on poller_get failing for poller_app is generated in
   libxl_ctx_init rather than passed through by libxl_poller_init
   from libxl__pipe_nonblock.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
(cherry picked from commit aae3765206)
2015-08-12 12:39:57 +01:00
Ian Jackson 8c409135e6 libxl: poll: Make libxl__poller_get have only one success return path
In preparation for doing some more work on successful exit.

No functional change.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
(cherry picked from commit 6fc946bc55)
2015-08-12 12:28:24 +01:00
Ian Campbell 9a4c62515c tools: libxl: Handle failure to create qemu dm logfile
If libxl_create_logfile fails for some reason then
libxl__create_qemu_logfile previously just carried on and dereferenced
the uninitialised logfile.

Check for the error from libxl_create_logfile, which has already
logged for us.

This was reported as Debian bug #784880.

Reported-by: Russell Coker <russell@coker.com.au>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: 784880@bugs.debian.org
Acked-by: Wei Liu <wei.liu2@citrix.com>
(cherry picked from commit e9d3a85997)
2015-08-12 12:26:35 +01:00
Ian Jackson 6040b3aeb3 xl: Sane handling of extra config file arguments
Various xl sub-commands take additional parameters containing = as
additional config fragments.

The handling of these config fragments has a number of bugs:

 1. Use of a static 1024-byte buffer.  (If truncation would occur,
    with semi-trusted input, a security risk arises due to quotes
    being lost.)

 2. Mishandling of the return value from snprintf, so that if
    truncation occurs, the to-write pointer is updated with the
    wanted-to-write length, resulting in stack corruption.  (This is
    XSA-137.)

 3. Clone-and-hack of the code for constructing the appended
    config file.

These are fixed here, by introducing a new function
`string_realloc_append' and using it everywhere.  The `extra_info'
buffers are replaced by pointers, which start off NULL and are
explicitly freed on all return paths.

The separate variable which will become dom_info.extra_config is
abolished (which involves moving the clearing of dom_info).

Additional bugs I observe, not fixed here:

 4. The functions which now call string_realloc_append use ad-hoc
    error returns, with multiple calls to `return'.  This currently
    necessitates multiple new calls to `free'.

 5. Many of the paths in xl call exit(-rc) where rc is a libxl status
    code.  This is a ridiculous exit status `convention'.

 6. The loops for handling extra config data are clone-and-hacks.

 7. Once the extra config buffer is accumulated, it must be combined
    with the appropriate main config file.  The code to do this
    combining is clone-and-hacked too.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Tested-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian,campbell@citrix.com>
(cherry picked from commit dd84604f35)
2015-07-29 16:39:56 +01:00
Konrad Rzeszutek Wilk 0d8cbcad03 libxl: In libxl_set_vcpuonline check for maximum number of VCPUs against the cpumap.
There is no sense in trying to online (or offline) CPUs when the size of
cpumap is greater than the maximum number of VCPUs the guest can go to.

As such fail the operation if the count of CPUs to online is greater
than what the guest started with. For the offline case we do not
check (as the bits are unset in the cpumap) and let it go through.

We coalesce some of the underlying libxl_set_vcpuonline code
together which was duplicated in QMP and XenStore codepaths.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit d83bf9d224)

Conflicts:
	tools/libxl/libxl.c
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
2015-06-12 13:00:00 +01:00
Ian Jackson bf06e40002 libxl: event handling: ao_inprogress does waits while reports outstanding
libxl__ao_inprogress needs to check (like
libxl__ao_complete_check_progress_reports) that there are no
oustanding progress callbacks.

Otherwise it might happen that we would destroy the ao while another
thread has an outstanding callback its egc report queue.  The other
thread would then, in its egc_run_callbacks, touch the destroyed ao.

Instead, when this happens in libxl__ao_inprogress, simply run round
the event loop again.  The thread which eventually makes the callback
will spot our poller in the ao, and notify the poller, waking us up.

This fixes an assertion failure race seen with libvirt:
  libvirtd: libxl_event.c:1792: libxl__ao_complete_check_progress_reports: Assertion `ao->in_initiator' failed.
or (after "Add an assert to egc_run_callbacks")
  libvirtd: libxl_event.c:1338: egc_run_callbacks: Assertion `aop->ao->magic == 0xA0FACE00ul' failed.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
(cherry picked from commit f1335f0d7b)
2015-06-12 12:45:03 +01:00
Ian Jackson 97051bd952 libxl: event handling: Break out ao_work_outstanding
Break out the test in libxl__ao_complete_check_progress_reports, into
ao_work_outstanding, which reports false if either (i) the ao is still
ongoing or (ii) there is a progress report (perhaps on a different
thread's callback queue) which has yet to be reported to the
application.

No functional change.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
(cherry picked from commit 93699882d9)
2015-06-12 12:44:58 +01:00
Ian Jackson fa62913ea7 libxl: Domain destroy: fork
Call xc_domain_destroy in a subprocess.  That allows us to do so
asynchronously, rather than blocking the whole process calling libxl.

The changes in detail:

 * Provide an libxl__ev_child in libxl__domain_destroy_state, and
   initialise it in libxl__domain_destroy.  There is no possibility
   to `clean up' a libxl__ev_child, but there need to clean it up, as
   the control flow ensures that we only continue after the child has
   exited.

 * Call libxl__ev_child_fork at the right point and put the call to
   xc_domain_destroy and associated logging in the child.  (The child
   opens a new xenctrl handle because we mustn't use the parent's.)

 * Consequently, the success return path from domain_destroy_domid_cb
   no longer calls dis->callback.  Instead it simply returns.

 * We plumb the errorno value through the child's exit status, if it
   fits.  This means we normally do the logging only in the parent.

 * Incidentally, we fix the bug that we were treating the return value
   from xc_domain_destroy as an errno value when in fact it is a
   return value from do_domctl (in this case, 0 or -1 setting errno).

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit 188e9c541f)
2015-05-27 17:02:52 +01:00
Ian Jackson c9b13f32dc libxl: Domain destroy: unlock userdata earlier
Unlock the userdata before we actually call xc_domain_destroy.  This
leaves open the possibility that other libxl callers will see the
half-destroyed domain (with no devices, paused), but this is fine.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit 1c91d6fba2)
2015-05-27 17:02:48 +01:00
Ian Jackson 0b19348f3c libxl: In domain death search, start search at first domid we want
From: Ian Jackson <Ian.Jackson@eu.citrix.com>

When domain_death_xswatch_callback needed a further call to
xc_domain_getinfolist it would restart it with the last domain it
found rather than the first one it wants.

If it only wants one it will also only ask for one domain.  The result
would then be that it gets the previous domain again (ie, the previous
one to the one it wants), which still doesn't reveal the answer to the
question, and it would therefore loop again.

It's completely unclear to me why I thought it was a good idea to
start the xc_domain_getinfolist with the last domain previously found
rather than the first one left un-confirmed.  The code has been that
way since it was introduced.

Instead, start each xc_domain_getinfolist at the next domain whose
status we need to check.

We also need to move the test for !evg into the loop, we now need evg
to compute the arguments to getinfolist.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reported-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit 4783c99aab)
2015-05-27 17:02:41 +01:00
Ian Campbell 6616c4d6fe tools: libxl: Explicitly disable graphics backends on qemu cmdline
By default qemu will try to create some sort of backend for the
emulated VGA device, either SDL or VNC.

However when the user specifies sdl=0 and vnc=0 in their configuration
libxl was not explicitly disabling either backend, which could lead to
one unexpectedly running.

If either sdl=1 or vnc=1 is configured then both before and after this
change only the backends which are explicitly enabled are configured,
i.e. this issue only occurs when all backends are supposed to have
been disabled.

This affects qemu-xen and qemu-xen-traditional differently.

If qemu-xen was compiled with SDL support then this would result in an
SDL window being opened if $DISPLAY is valid, or a failure to start
the guest if not. Passing "-display none" to qemu before any further
-sdl options disables this default behaviour and ensures that SDL is
only started if the libxl configuration demands it.

If qemu-xen was compiled without SDL support then qemu would instead
start a VNC server listening on ::1 (IPv6 localhost) or 127.0.0.1
(IPv4 localhost) with IPv6 preferred if available. Explicitly pass
"-vnc none" when vnc is not enabled in the libxl configuration to
remove this possibility.

qemu-xen-traditional would never start a vnc backend unless asked.
However by default it will start an SDL backend, the way to disable
this is to pass a -vnc option. In other words passing "-vnc none" will
disable both vnc and sdl by default. sdl can then be reenabled if
configured by subsequent use of the -sdl option.

Tested with both qemu-xen and qemu-xen-traditional built with SDL
support and:
	xl cr # defaults
	xl cr sdl=0 vnc=0
	xl cr sdl=1 vnc=0
	xl cr sdl=0 vnc=1
	xl cr sdl=0 vnc=0 vga=\"none\"
	xl cr sdl=0 vnc=0 nographic=1
with both valid and invalid $DISPLAY.

This is XSA-119 / CVE-2015-2152.

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 91b0ae9db3)
2015-03-12 13:47:23 +00:00
Jim Fehlig 6302c61f4f Install libxlutil.h
libxlutil.{a,so} are installed, but not the corresponding header
file.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
(cherry picked from commit 8ff0798036)
2015-02-16 14:44:36 +00:00
Julien Grall 9ae1853b03 libxl: Don't ignore error when we fail to give access to ioport/irq/iomem
If we fail to give the access, the domain will unlikely work correctly.
So we should bail out at the first error.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
(cherry picked from commit 7070eec417)
2015-01-19 17:46:17 +00:00
Ed Swierk 7ba4cdfadd libxl: Fix building libxlu_cfg_y.y with bison 3.0
- Use %lex-param instead of obsolete YYLEX_PARAM to override lex scanner
  parameter
- Change deprecated %name-prefix= to %name-prefix

Tested against bison 2.4.1 and 3.0.2.

This is expected to sometimes (depending on timestamps and whether the
bison input files are edited) break building on systems with ancient
versions of bison.  Bison 2.4.1 is known to work and was released in
December 2008.

Also, consquentially, regenerate bison output files with bison
1:2.5.dfsg-2.1 from Debian wheezy.

Signed-off-by: Ed Swierk <eswierk@skyportsystems.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Tested-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
2015-01-06 15:28:17 +00:00
Ian Jackson 3bd8559e5e libxl: Renegerate flex output files
Regenerate libxl_*_l.* with flex 2.5.35-10.1 as in current Debian
wheezy.  The differences are trivial: addition of declarations of
xlu__cfg_yyget_column and xlu__cfg_yyset_column, but no code body
changes.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
2015-01-06 15:28:17 +00:00
Stefano Stabellini 8233ca53ad reset PCI devices on force removal even when QEMU returns error
On do_pci_remove when QEMU returns error, we just bail out early without
resetting the device. On domain shutdown we are racing with QEMU exiting
and most often QEMU closes the QMP connection before executing the
requested command.

In these cases if force=1, it makes sense to go ahead with rest of the
PCI device removal, that includes resetting the device and calling
xc_deassign_device. Otherwise we risk not resetting the device properly
on domain shutdown.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
2015-01-06 11:15:45 +00:00
Ian Campbell 1ec4e3336d tools: libxl: link libxlu against libxl.
It uses libxl_defbool_set and must therefore be linked against the
right library.

Spotted by dpkg-shlibdeps and pointed out by Bastian Blank:

dpkg-shlibdeps: warning: symbol libxl_defbool_set used by debian/libxen-4.4/usr/lib/libxlutil-4.4.so found in none of the libraries

This required switching the make rule from $^ to an explicit
LIBXLU_OBJS since the former now includes libxenlight.so.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Cc: Bastian Blank <waldi@debian.org>
2015-01-06 11:13:44 +00:00
Andrew Cooper 54eb18cd63 tools/libxl: Use of init()/dispose() to avoid leaking libxl_dominfo.ssid_label
libxl_dominfo contains a ssid_label pointer which will have memory allocated
for it in libxl_domain_info() if the hypervisor has CONFIG_XSM compiled.
However, the lack of appropriate use of libxl_dominfo_{init,dispose}() will
cause the label string to be leaked, even in success cases.

This was discovered by XenServers Coverity scanning, and are issues not
identified by upstream Coverity Scan.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
2015-01-06 11:10:43 +00:00
Ian Jackson 7ca2558c04 libxl: Fix if{} nesting in do_pci_remove
do_pci_remove contained this:

    if (type == LIBXL_DOMAIN_TYPE_HVM) {
       [stuff]
    } else if (type != LIBXL_DOMAIN_TYPE_PV)
        abort();
    {

This is bizarre, and not correct.  The effect is that HVM guests end
up running both the proper code and that intended for PV guests.  This
causes (amongst other things) trouble when PCI devices are
hot-unplugged from HVM guests.

This bug was introduced in abfb006f "tools/libxl: explicitly grant
access to needed I/O-memory ranges".

This is clear candidate for Xen 4.5, being a bugfix to an important
feature.

Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Tested-by: Robert Hu <robert.hu@intel.com>
Rlease-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
CC: Sander Eikelenboom <linux@eikelenboom.it>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
2015-01-05 16:45:13 +00:00
Ian Jackson 6b511af686 libxl: Initialise CTX->xce in domain suspend, as needed
When excuting xl migrate/Remus, the following error can occur:
  [root@master xen]# xl migrate 5 slaver
  migration target: Ready to receive domain.
  Saving to migration stream new xl format (info 0x1/0x0/1225)
  Loading new save file <incoming migration stream> (new xl fmt info 0x1/0x0/12\
)
   Savefile contains xl domain config in JSON format
  Parsing config from <saved>
  Segmentation fault (core dumped)

This is because CTX->xce is used without been initialized.
The bug was introduced by commit 2ffeb5d7f5
    libxl: events: Deregister evtchn fd when not needed
which removed the initialization of xce from libxl__ctx_alloc.

In this patch we initialise the CTX->xce before using it.  Also, we
adjust the doc comment for libxl__ev_evtchn_* to mention the need to
do so.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Wei Liu <wei.liu2@citrix.com>
2015-01-05 16:42:11 +00:00
George Dunlap 7e88c23239 libxl: Tell qemu to use raw format when using a tapdisk
At the moment libxl unconditinally passes the underlying file format
to qemu in the device string.  However, when tapdisk is in use,
tapdisk handles the underlying format and presents qemu with
effectively a raw disk.  When qemu looks at the tapdisk block device
and doesn't find the image format it was looking for, it will fail.

This effectively means that tapdisk cannot be used with HVM domains at
the moment except for raw files.

Instead, if we're using a tapdisk backend, tell qemu to use a raw file
format.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
[ ijc -- nuked extra blank line ]
2014-12-16 12:47:41 +00:00
Wei Liu 09b7ff1a11 xl: print message to stdout when (!debug && dryrun)
In commit d36a3734a ("xl: fix migration failure with xl migrate
--debug"), message is printed to stderr for both debug mode
and dryrun mode. That caused rdname() in xendomains fails to parse
domain name since it's expecting input from xl's stdout.

So this patch separates those two cases. If xl is running in debug mode,
then message is printed to stderr; if xl is running in dryrun mode and
debug is not enabled, message is printed to stdout. This will fix
xendomains and other scripts that use "xl create --dryrun", as well as
not re-introducing the old bug fixed in d36a3734a.

Reported-by: Mark Pryor <tlviewer@yahoo.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: M A Young <m.a.young@durham.ac.uk>
Cc: Ian Campbell <ian.campbell@citrix.com>
Release-Acked-by: Konrad Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
2014-12-16 12:46:17 +00:00
Ian Jackson 60ce518a1b libxl: events: Document and enforce actual callbacks restriction
libxl_event_register_callbacks cannot reasonably be called while libxl
is busy (has outstanding operations and/or enabled events).

This is because the previous spec implied (although not entirely
clearly) that event hooks would not be called for existing fd and
timeout interests.  There is thus no way to reliably ensure that libxl
would get told about fds and timeouts which it became interested in
beforehand.

So there have to be no such fds or timeouts, which means that the
callbacks must only be registered or changed when the ctx is idle.

Document this restriction, and enforce it with a pair of asserts.

(It would be nicer, perhaps, to say that the application may not call
libxl_osevent_register_hooks other than right after creating the ctx.
But there are existing callers, including libvirt, who do it later -
even after doing major operations such as domain creation.)

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Tested-by: Ian Campbell <ian.campbell@citrix.com>
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
2014-12-10 13:48:31 +00:00
Ian Jackson 2ffeb5d7f5 libxl: events: Deregister evtchn fd when not needed
We want to have no fd events registered when we are idle.
In this patch, deal with the evtchn fd:

 * Defer setup of the evtchn handle to the first use.
 * Defer registration of the evtchn fd; register as needed on use.
 * When cancelling an evtchn wait, or when wait setup fails, check
   whether there are now no evtchn waits and if so deregister the fd.
 * On libxl teardown, the evtchn fd should therefore be unregistered.
   assert that this is the case.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Tested-by: Ian Campbell <ian.campbell@citrix.com>
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
v2: Do not bother putting evtchn_fd in the ctx; instead, get it
     from xc_evtchn_fd when we need it.  (Cosmetic.)
    Do not register the evtchn fd multiple times: check it's not
     registered before we call libxl__ev_fd_register.  (Bugfix.)
2014-12-10 13:48:31 +00:00
Ian Jackson 5a96825745 libxl: events: Tear down SIGCHLD machinery on ctx destruction
We want to have no fd events registered when we are idle.
Also, we should put back the default SIGCHLD handler.  So:

 * In libxl_ctx_free, use libxl_childproc_setmode to set the mode to
   the default, which is libxl_sigchld_owner_libxl (ie `libxl owns
   SIGCHLD only when it has active children').

   But of course there are no active children at libxl teardown so
   this results in libxl__sigchld_notneeded: the ctx loses its
   interest in SIGCHLD (unsetting the SIGCHLD handler if we were the
   last ctx) and deregisters the per-ctx selfpipe fd.

 * assert that this is the case: ie that we are no longer interested
   in the selfpipe.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Tested-by: Ian Campbell <ian.campbell@citrix.com>
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
2014-12-10 13:48:31 +00:00
Ian Jackson 4b9143e4a4 libxl: events: Deregister, don't just modify, sigchld pipe fd
We want to have no fd events registered when we are idle.  This
implies that we must be able to deregister our interest in the sigchld
self-pipe fd, not just modify to request no events.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Tested-by: Ian Campbell <ian.campbell@citrix.com>
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
2014-12-10 13:48:31 +00:00
Ian Jackson 77a1bf3779 libxl: events: Deregister xenstore watch fd when not needed
We want to have no fd events registered when we are idle.
In this patch, deal with the xenstore watch fd:

* Track the total number of active watches.
* When deregistering a watch, or when watch registration fails, check
  whether there are now no watches and if so deregister the fd.
* On libxl teardown, the watch fd should therefore be unregistered.
  assert that this is the case.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Tested-by: Ian Campbell <ian.campbell@citrix.com>
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
2014-12-10 13:48:31 +00:00
Ian Jackson f49f9b4114 libxl: events: Assert that libxl_ctx_free is not called from a hook
No-one in their right mind would do this, and if they did everything
would definitely collapse.  Arrange that if this happens, we crash
ASAP.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Tested-by: Ian Campbell <ian.campbell@citrix.com>
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
2014-12-10 13:48:30 +00:00
Wei Liu bfac2d7a00 libxl: expose #define to 4.5 and above
In e3abab74 (libxl: un-constify return value of libxl_basename), the
macro was exposed to releases < 4.5. However only new code is able to
make use of that macro so it should be exposed to releases >= 4.5.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
2014-12-04 11:53:43 +00:00
Wei Liu 4d1a77ba7a xl: fix two memory leaks
Free strings returned by libxl_basename after used.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
[ ijc -- s/basename/kernel_basename in parse_config_data to avoid
         shadowing basename(3). ]
2014-12-02 15:21:16 +00:00
Euan Harris 276ba78062 libxl: Don't dereference null new_name pointer in libxl_domain_rename()
libxl__domain_rename() unconditionally dereferences its new_name
parameter, to check whether it is an empty string.   Add a check to
avoid a segfault if new_name is null.

Signed-off-by: Euan Harris <euan.harris@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
2014-12-02 14:18:57 +00:00
Wei Liu e3abab7459 libxl: un-constify return value of libxl_basename
The string returned is malloc'ed but marked as "const".

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
2014-12-02 13:55:28 +00:00
Chunyan Liu e609bb983e missing chunk of HVM direct kernel boot patch
Found by Stefano, this chunk of the patch was never applied to
xen-unstable (commit 11dffa2359),
see http://marc.info/?l=qemu-devel&m=140471493425353&w=2.

Signed-off-by: Chunyan Liu <cyliu@suse.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
2014-12-02 13:54:41 +00:00
M A Young d36a3734a6 xl: fix migration failure with xl migrate --debug
Migrations with xl migrate --debug will fail because debugging
information from the receiving process is written to the stdout
channel. This channel is also used for status messages so the
migration will fail as the sending process receives an unexpected
message. This patch moves the debugging information to the stderr
channel.

Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
2014-12-02 13:48:54 +00:00
Euan Harris 1fed4af8d8 libxl: libxl_domain_info: fix typo in error message
Signed-off-by: Euan Harris <euan.harris@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
2014-12-02 13:43:54 +00:00
Zhigang Wang 188336bb86 set pv guest default video_memkb to 0
Before this patch, pv guest video_memkb is -1, which is an invalid value.
And it will cause the xenstore 'memory/targe' calculation wrong:

    memory/target = info->target_memkb - info->video_memkb

Signed-off-by: Zhigang Wang <zhigang.x.wang@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
2014-11-28 11:23:58 +00:00
Chunyan Liu 412f0e6401 fix rename: xenstore not fully updated
libxl__domain_rename only updates /local/domain/<domid>/name,
/vm/<uuid>/name in xenstore are not updated. Add code in
libxl__domain_rename to update /vm/<uuid>/name too.

Signed-off-by: Chunyan Liu <cyliu@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2014-11-28 11:22:13 +00:00
Chunyan Liu 8722dd6c59 remove domain field in xenstore backend dir
Remove the unusual 'domain' field under backend directory. The
affected are backend/console, backend/vfb, backend/vkbd.

The correct way to obtain a domain's name is via
libxl_domid_to_name(), or by reading from /local/domain/$DOMID/name
for toolstacks not using libxl.

Signed-off-by: Chunyan Liu <cyliu@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
[ ijc -- added second paragraph to commit message ]
2014-11-28 11:21:13 +00:00
Wei Liu 1c098487d3 libxl: allow copying between bitmaps of different sizes
When parsing bitmap objects JSON parser will create libxl_bitmap map of the
smallest size needed.

This can cause problems when saved image file specifies CPU affinity.  For
example, if 'vcpu_hard_affinity' in the saved image has only the first CPU
specified, just a single byte will be allocated and libxl_bitmap->size will be
set to 1.

This will result in assertion in libxl_set_vcpuaffinity()->libxl_bitmap_copy()
since the destination bitmap is created for maximum number of CPUs.

We could allocate that bitmap of the same size as the source, however, it is
later passed to xc_vcpu_setaffinity() which expects it to be sized to the max
number of CPUs

To fix this issue, introduce an internal function to allowing copying between
bitmaps of different sizes. Note that this function is only used in
libxl_set_vcpuaffinity at the moment. Though NUMA placement logic invoke
libxl_bitmap_copy as well there's no need to replace those invocations.  NUMA
placement logic comes into effect when no vcpu / node pinning is provided, so
it always operates on bitmap of the same sizes (that is, size of maximum
number of cpus /nodes).

Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
2014-11-28 11:01:00 +00:00
George Dunlap 47c6e7f28d xl: Return proper error codes for block-attach and block-detach
Return proper error codes on failure so that scripts can tell whether
the command completed properly or not.

This is not a proper fix, since it fails to call
libxl_device_disk_dispose() on the error path.  But a proper fix
requires some refactoring, so given where we are in the release
process, it's better to have a fix that is simple and obvious, and do
the refactoring once the next development window opens up.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
2014-11-25 14:04:04 +00:00
Euan Harris 405254bbb2 libxl: Document device parameter of libxl_device_<type>_add functions
The device parameter of libxl_device_<type>_add is an in/out parameter.
Unspecified fields are filled in with appropriate values for the created
device when the function returns.  Document this behaviour.

Signed-off-by: Euan Harris <euan.harris@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2014-11-20 15:31:50 +00:00