Re: [Xen-devel] [RFC][PATCH 08/13] tools: extend xc_assign_device() to support rdm reservation policy
On 2015/5/11 18:53, Jan Beulich wrote: On 11.05.15 at 11:45, wrote: On 2015/4/20 21:39, Jan Beulich wrote: On 10.04.15 at 11:21, wrote: --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -1654,13 +1654,15 @@ int xc_domain_setdebugging(xc_interface *xch, int xc_assign_device( xc_interface *xch, uint32_t domid, -uint32_t machine_sbdf) +uint32_t machine_sbdf, +uint32_t flag) { DECLARE_DOMCTL; domctl.cmd = XEN_DOMCTL_assign_device; domctl.domain = domid; domctl.u.assign_device.machine_sbdf = machine_sbdf; +domctl.u.assign_device.sbdf_flag = flag; The previous patch needs to initialize this field, in order to not pass random input to the hypervisor. Using the ..._TRY value here intermediately (until this patch gets applied) would seem the right approach. If I'm correct, looks I should introduce a little of change in previous patch, diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index 250d1e4..0bcfd87 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -1513,7 +1513,7 @@ int iommu_do_pci_domctl( { u16 seg; u8 bus, devfn; -u32 flag; +u32 flag = XEN_DOMCTL_PCIDEV_RDM_TRY; Provided that constant is available already at that point (I didn't Yes, both that definition and this usage are in the previous patch. Thanks Tiejun check); if it isn't, you'd probably want to go with plain zero. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH Remus v3 2/3] libxc/restore: add checkpointed flag to the restore context
On 14/05/2015 02:09, Yang Hongyang wrote: > > > On 05/13/2015 11:15 PM, Andrew Cooper wrote: >> On 13/05/15 09:35, Yang Hongyang wrote: >>> add checkpointed flag to the restore context. >>> >>> Signed-off-by: Yang Hongyang >>> CC: Ian Campbell >>> CC: Ian Jackson >>> CC: Wei Liu >>> CC: Andrew Cooper >> >> This appears unused, given the new restore code. > > This is used by handle_checkpoint() for the sanity check. Ah yes, and I have realised that it will need to be used when I fix up the qemu record handling in the future, so Reviewed-by: Andrew Cooper > >> >> ~Andrew >> >>> --- >>> tools/libxc/xc_sr_common.h | 3 +++ >>> tools/libxc/xc_sr_restore.c | 1 + >>> 2 files changed, 4 insertions(+) >>> >>> diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h >>> index 0ba9728..f8121e7 100644 >>> --- a/tools/libxc/xc_sr_common.h >>> +++ b/tools/libxc/xc_sr_common.h >>> @@ -205,6 +205,9 @@ struct xc_sr_context >>> uint32_t guest_type; >>> uint32_t guest_page_size; >>> >>> +/* Plain VM, or checkpoints over time. */ >>> +bool checkpointed; >>> + >>> /* >>>* Xenstore and Console parameters. >>>* INPUT: evtchn & domid >>> diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c >>> index 8022c3d..0e512ec 100644 >>> --- a/tools/libxc/xc_sr_restore.c >>> +++ b/tools/libxc/xc_sr_restore.c >>> @@ -604,6 +604,7 @@ int xc_domain_restore2(xc_interface *xch, int >>> io_fd, uint32_t dom, >>> ctx.restore.console_domid = console_domid; >>> ctx.restore.xenstore_evtchn = store_evtchn; >>> ctx.restore.xenstore_domid = store_domid; >>> +ctx.restore.checkpointed = checkpointed_stream; >>> ctx.restore.callbacks = callbacks; >>> >>> IPRINTF("In experimental %s", __func__); >> >> . >> > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH Remus v3 3/3] libxc/restore: implement Remus checkpointed restore
On 14/05/2015 06:42, Yang Hongyang wrote: > >>> @@ -541,7 +618,27 @@ static int restore(struct xc_sr_context *ctx) >>> { >>> rc = read_record(ctx, &rec); >>> if ( rc ) >>> -goto err; >>> +{ >>> +if ( ctx->restore.buffer_all_records ) >>> +goto err_buf; >> >> Please can we choose a label sufficiently different to "err". >> >> "resume_from_checkpoint" perhaps? > > I think "remus_failover" is better? If you don't have objections, I will > use it as the label. Fine by me. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [qemu-upstream-4.3-testing test] 55875: regressions - FAIL
flight 55875 qemu-upstream-4.3-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/55875/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail REGR. vs. 50282 test-amd64-amd64-xl-qemuu-winxpsp3 15 guest-localmigrate/x10 fail REGR. vs. 50282 Regressions which are regarded as allowable (not blocking): test-amd64-i386-freebsd10-amd64 13 guest-localmigrate fail REGR. vs. 50282 test-amd64-i386-xl-qemuu-win7-amd64 12 guest-localmigrate fail REGR. vs. 50282 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 16 guest-stop fail never pass version targeted for testing: qemuud7b34893e0ad5c84d898b34fda8a465dfd7e8376 baseline version: qemuu7f34050dc014ae8f4078d48aec97ec6553151bf2 People who touched revisions under test: Petr Matousek jobs: build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass test-amd64-i386-xl pass test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-i386-freebsd10-amd64 fail test-amd64-amd64-xl-qemuu-ovmf-amd64 fail test-amd64-i386-xl-qemuu-ovmf-amd64 fail test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-amd64-xl-credit2 pass test-amd64-i386-freebsd10-i386 pass test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-amd64-libvirt pass test-amd64-i386-libvirt pass test-amd64-amd64-xl-multivcpupass test-amd64-amd64-pairpass test-amd64-i386-pair pass test-amd64-amd64-xl-sedf-pin pass test-amd64-amd64-pv pass test-amd64-i386-pv pass test-amd64-amd64-xl-sedf pass test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 fail test-amd64-amd64-xl-qemuu-winxpsp3 fail sg-report-flight on osstest.test-lab.xenproject.org logs: /home/osstest/pub/logs images: /home/osstest/pub/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. commit d7b34893e0ad5c84d898b34fda8a465dfd7e8376 Author: Petr Matousek Date: Wed May 6 09:48:59 2015 +0200 fdc: force the fifo access to be in bounds of the allocated buffer During processing of certain commands such as FD_CMD_READ_ID and FD_CMD_DRIVE_SPECIFICATION_COMMAND the fifo memory access could get out of bounds leading to memory corruption with values coming from the guest. Fix this by making sure that the index is always bounded by the allocated memory. This is CVE-2015-3456. Signed-off-by: Petr Matousek Reviewed-by: John Snow ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC][PATCH 04/13] tools/libxl: detect and avoid conflicts with RDM
On 2015/5/11 19:32, Wei Liu wrote: On Mon, May 11, 2015 at 04:09:53PM +0800, Chen, Tiejun wrote: On 2015/5/8 22:43, Wei Liu wrote: Sorry for the late review. This series fell through the crack. Thanks for your review. On Fri, Apr 10, 2015 at 05:21:55PM +0800, Tiejun Chen wrote: While building a VM, HVM domain builder provides struct hvm_info_table{} to help hvmloader. Currently it includes two fields to construct guest e820 table by hvmloader, low_mem_pgend and high_mem_pgend. So we should check them to fix any conflict with RAM. RMRR can reside in address space beyond 4G theoretically, but we never see this in real world. So in order to avoid breaking highmem layout How does this break highmem layout? In most cases highmen is always continuous like [4G, ...] but RMRR is theoretically beyond 4G but very rarely. Especially we don't see this happened in real world. So we don't want to such a case breaking the highmem. The problem is we take this approach just because this rarely happens *now* is not future proof. It needs to be clearly documented somewhere in the manual (or any other Intel docs) and be referenced in the code. Otherwise we might end up in a situation that no-one knows how it is supposed to work and no-one can fix it if it breaks in the future, that is, every single device on earth requires RMRR > 4G overnight (I'm exaggerating). Or you can just make it works with highmem. How much more work do you envisage? (If my above comment makes no sense do let me know. I only have very shallow understanding of RMRR) Maybe I'm misleading you :) I don't mean RMRR > 4G is not allowed to work in our implementation. What I'm saying is that our *policy* is just simple for this kind of rare highmem case... we don't solve highmem conflict. Note this means highmem rmrr could still be supported if no conflict. Like these two sentences above. Aren't you actively trying to avoid conflict in libxl? RMRR is fixed by BIOS so we can't aovid conflict. Here we want to adopt some good policies to address RMRR. In the case of highmemt, that simple policy should be enough? Whatever policy you and HV maintainers agree on. Just clearly document it. Do you mean I should brief this patch description into one separate document? But in the case of lowmem, RMRR probably scatter the whole RAM space. Especially multiple RMRR entries would worsen this to lead a complicated memory layout. And then its hard to extend hvm_info_table{} to work hvmloader out. So here we're trying to figure out a simple solution to avoid breaking existing layout. So when a conflict occurs, #1. Above a predefined boundary (default 2G) - move lowmem_end below reserved region to solve conflict; I hope this "predefined boundary" is user tunable. I will check later in this patch if it is the case. Yes. As we clarified in that comments, * TODO: Its batter to provide a config parameter for this boundary value. This mean I would provide a patch address this since currently I just think this is not a big deal? Yes please provide a config option to override that. It's reasonable that user wants to change that. Okay. #2 Below a predefined boundary (default 2G) - Check force/try policy. "force" policy leads to fail libxl. Note when both policies are specified on a given region, 'force' is always preferred. "try" policy issue a warning message and also mask this entry INVALID to indicate we shouldn't expose this entry to hvmloader. Signed-off-by: Tiejun Chen --- tools/libxc/include/xenctrl.h | 8 ++ tools/libxc/include/xenguest.h | 3 +- tools/libxc/xc_domain.c| 40 + tools/libxc/xc_hvm_build_x86.c | 28 +++--- tools/libxl/libxl_create.c | 2 +- tools/libxl/libxl_dm.c | 195 + tools/libxl/libxl_dom.c| 27 +- tools/libxl/libxl_internal.h | 11 ++- tools/libxl/libxl_types.idl| 7 ++ 9 files changed, 303 insertions(+), 18 deletions(-) diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 59bbe06..299b95f 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -2053,6 +2053,14 @@ int xc_get_device_group(xc_interface *xch, uint32_t *num_sdevs, uint32_t *sdev_array); +struct xen_reserved_device_memory +*xc_device_get_rdm(xc_interface *xch, + uint32_t flag, + uint16_t seg, + uint8_t bus, + uint8_t devfn, + unsigned int *nr_entries); + int xc_test_assign_device(xc_interface *xch, uint32_t domid, uint32_t machine_sbdf); [...] uint64_t mem_target; /* Memory target in bytes. */ uint64_t mmio_size; /* Size of the MMIO hole in bytes. */ const char *ima
[Xen-devel] [qemu-upstream-4.2-testing test] 55877: tolerable FAIL - PUSHED
flight 55877 qemu-upstream-4.2-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/55877/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-amd64-i386-xl-qemuu-win7-amd64 12 guest-localmigrate fail blocked in 36332 test-amd64-amd64-xl-qemuu-winxpsp3 15 guest-localmigrate/x10 fail blocked in 36332 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 15 guest-localmigrate/x10 fail blocked in 36332 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail never pass test-amd64-amd64-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass test-amd64-i386-xend-qemuu-winxpsp3 20 leak-check/checkfail never pass test-i386-i386-xl-qemuu-winxpsp3 16 guest-stop fail never pass version targeted for testing: qemuu35fc1ed1d479528c1601c1bc65628fb8ab6aae52 baseline version: qemuue49807b61c8152b0730a310a3d771ac253e750aa People who touched revisions under test: Petr Matousek jobs: build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-amd64-xl-qemuu-ovmf-amd64 fail test-amd64-i386-xl-qemuu-ovmf-amd64 fail test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 fail test-amd64-i386-xend-qemuu-winxpsp3 fail test-amd64-amd64-xl-qemuu-winxpsp3 fail test-i386-i386-xl-qemuu-winxpsp3 fail sg-report-flight on osstest.test-lab.xenproject.org logs: /home/osstest/pub/logs images: /home/osstest/pub/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : + branch=qemu-upstream-4.2-testing + revision=35fc1ed1d479528c1601c1bc65628fb8ab6aae52 + . cri-lock-repos ++ . cri-common +++ . cri-getconfig +++ umask 002 +++ getconfig Repos +++ perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x '!=' x/home/osstest/repos/lock ']' ++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock ++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push qemu-upstream-4.2-testing 35fc1ed1d479528c1601c1bc65628fb8ab6aae52 + branch=qemu-upstream-4.2-testing + revision=35fc1ed1d479528c1601c1bc65628fb8ab6aae52 + . cri-lock-repos ++ . cri-common +++ . cri-getconfig +++ umask 002 +++ getconfig Repos +++ perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']' + . cri-common ++ . cri-getconfig ++ umask 002 + select_xenbranch + case "$branch" in + tree=qemuu + xenbranch=xen-4.2-testing + '[' xqemuu = xlinux ']' + linuxbranch= + '[' x = x ']' + qemuubranch=qemu-upstream-4.2-testing + : tested/2.6.39.x + . ap-common ++ : osst...@xenbits.xen.org +++ getconfig OsstestUpstream +++ perl -e ' use Osstest; readglobalconfig(); print $c{"OsstestUpstream"} or die $!; ' ++ : ++ : git://xenbits.xen.org/xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/xen.git ++ : git://xenbits.xen.org/staging/qemu-xen-4.2-testing.git ++ : git://git.kernel.org ++ : git://git.kernel.org/pub/scm/linux/kernel/git ++ : git ++ : git://xenbits.xen.org/libvirt.git ++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git ++ : git://xenbits.xen.org/libvirt.git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git +++ besteffort_repo ht
[Xen-devel] [PATCH V2] xen/vm_event: Clean up control-register-write vm_events
As suggested by Andrew Cooper, this patch attempts to remove some redundancy and allow for an easier time when adding vm_events for new control registers in the future, by having a single VM_EVENT_REASON_WRITE_CTRLREG vm_event type, meant to serve CR0, CR3, CR4 and (newly introduced) XCR0. The actual control register will be deduced by the new .index field in vm_event_write_ctrlreg (renamed from vm_event_mov_to_cr). The patch has also modified the xen-access.c test - it is now able to log CR3 events. Signed-off-by: Razvan Cojocaru --- Changes since V1: - VM_EVENT_REASON_MOV_TO_CR became VM_EVENT_REASON_WRITE_CTRLREG. - xc_monitor_mov_to_cr() became xc_monitor_write_ctrlreg(). - XEN_DOMCTL_MONITOR_EVENT_MOV_TO_CR became XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG. - Plugged in an actual event for VM_EVENT_X86_XCR0. - X86_CR0 became VM_EVENT_X86_CR0, and so on. --- tools/libxc/include/xenctrl.h |9 ++ tools/libxc/xc_monitor.c| 40 +++ tools/tests/xen-access/xen-access.c | 30 -- xen/arch/x86/hvm/event.c| 50 +++-- xen/arch/x86/hvm/hvm.c |9 -- xen/arch/x86/hvm/vmx/vmx.c |4 +-- xen/arch/x86/monitor.c | 60 --- xen/include/asm-x86/domain.h| 20 xen/include/asm-x86/hvm/event.h |4 +-- xen/include/public/domctl.h | 12 +++ xen/include/public/vm_event.h | 27 +--- 11 files changed, 105 insertions(+), 160 deletions(-) diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index a689caf..d6008ff 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -2355,12 +2355,9 @@ int xc_mem_access_disable_emulate(xc_interface *xch, domid_t domain_id); void *xc_monitor_enable(xc_interface *xch, domid_t domain_id, uint32_t *port); int xc_monitor_disable(xc_interface *xch, domid_t domain_id); int xc_monitor_resume(xc_interface *xch, domid_t domain_id); -int xc_monitor_mov_to_cr0(xc_interface *xch, domid_t domain_id, bool enable, - bool sync, bool onchangeonly); -int xc_monitor_mov_to_cr3(xc_interface *xch, domid_t domain_id, bool enable, - bool sync, bool onchangeonly); -int xc_monitor_mov_to_cr4(xc_interface *xch, domid_t domain_id, bool enable, - bool sync, bool onchangeonly); +int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id, + uint16_t index, bool enable, bool sync, + bool onchangeonly); int xc_monitor_mov_to_msr(xc_interface *xch, domid_t domain_id, bool enable, bool extended_capture); int xc_monitor_singlestep(xc_interface *xch, domid_t domain_id, bool enable); diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c index 87ad968..63013de 100644 --- a/tools/libxc/xc_monitor.c +++ b/tools/libxc/xc_monitor.c @@ -45,8 +45,9 @@ int xc_monitor_resume(xc_interface *xch, domid_t domain_id) NULL); } -int xc_monitor_mov_to_cr0(xc_interface *xch, domid_t domain_id, bool enable, - bool sync, bool onchangeonly) +int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id, + uint16_t index, bool enable, bool sync, + bool onchangeonly) { DECLARE_DOMCTL; @@ -54,39 +55,8 @@ int xc_monitor_mov_to_cr0(xc_interface *xch, domid_t domain_id, bool enable, domctl.domain = domain_id; domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE : XEN_DOMCTL_MONITOR_OP_DISABLE; -domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_MOV_TO_CR0; -domctl.u.monitor_op.u.mov_to_cr.sync = sync; -domctl.u.monitor_op.u.mov_to_cr.onchangeonly = onchangeonly; - -return do_domctl(xch, &domctl); -} - -int xc_monitor_mov_to_cr3(xc_interface *xch, domid_t domain_id, bool enable, - bool sync, bool onchangeonly) -{ -DECLARE_DOMCTL; - -domctl.cmd = XEN_DOMCTL_monitor_op; -domctl.domain = domain_id; -domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE -: XEN_DOMCTL_MONITOR_OP_DISABLE; -domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_MOV_TO_CR3; -domctl.u.monitor_op.u.mov_to_cr.sync = sync; -domctl.u.monitor_op.u.mov_to_cr.onchangeonly = onchangeonly; - -return do_domctl(xch, &domctl); -} - -int xc_monitor_mov_to_cr4(xc_interface *xch, domid_t domain_id, bool enable, - bool sync, bool onchangeonly) -{ -DECLARE_DOMCTL; - -domctl.cmd = XEN_DOMCTL_monitor_op; -domctl.domain = domain_id; -domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE -: XEN_DOMCTL_MONITOR_OP_DISABLE; -domctl.u.monitor_op.ev
Re: [Xen-devel] [Pkg-xen-devel] Bug#785187: xen-hypervisor-4.5-amd64: Option ucode=scan is not working
Thanks Atom2, thanks for the refs, I've added the Debian bug and the submitter back to the CC. On Wed, 2015-05-13 at 22:11 +0200, Atom2 wrote: > Am 13.05.15 um 15:41 schrieb Ian Campbell: > > I think I remember some discussion of something in this area not too > > long ago on xen-devel. CC-s added. > I assume you refer to this discussion which happend to be on the > xen-users mailing list: > http://lists.xen.org/archives/html/xen-users/2014-05/msg00052.html > > Especially look at the first answer > (http://lists.xen.org/archives/html/xen-users/2014-05/msg00053.html). This says "not 'cpio -o c' as some information on the internet suggests", do you have a link? Is it to something the xenproject controls and could update (i.e. our wiki and/or in tree docs?) Cheers, Ian. > Following those points mentioned in there should get you up and running > - unless there's really a bug lurking somewhere. > > My guess is that the cpio archive was not created with the (required) > cpio option "-H newc". > > Regards Atom2 > > Konrad, do you know of any issues with ucode=scan in 4.4? > > > > On Wed, 2015-05-13 at 10:27 +0200, Stephan Seitz wrote: > >> Package: xen-hypervisor-4.5-amd64 > >> Version: 4.5.0-1 > >> Severity: normal > >> > >> Dear Maintainer, > >> > >> according to the documentation the option ucode=scan should tell XEN to > >> look for a microcode update in an uncompressed initrd. > >> > >> While I don’t use the Debian kernel the tools to generate the initrd are > >> part of Debian. The command „cpio -i < /boot/initrd.img-4.0.2-Dom0” > >> creates the directory structure „kernel/x86/microcode/GenuineIntel.bin”, > >> so I think the initrd is allright. > >> > >> But according to /proc/cpuinfo the microcode update doesn’t happen. If > >> I copy GenuineIntel.bin to /boot, create a new module section in grub.cfg > >> and boot with ucode=-1, then the update is working. In both cases dmesg > >> or „xl dmesg” don’t tell anything about microcode updates (successfull or > >> not). > >> > >> The option ucode=scan doesn’t work with XEN 4.4 either or with 3.6 > >> kernels. > >> > >> -- System Information: > >> Debian Release: stretch/sid > >>APT prefers oldoldstable-updates > >>APT policy: (500, 'oldoldstable-updates'), (500, 'testing'), (500, > >> 'stable') > >> Architecture: amd64 (x86_64) > >> Foreign Architectures: i386 > >> > >> Kernel: Linux 4.0.2-Dom0 (SMP w/8 CPU cores) > >> Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8) > >> Shell: /bin/sh linked to /bin/dash > >> Init: sysvinit (via /sbin/init) > >> > >> xen-hypervisor-4.5-amd64 depends on no packages. > >> > >> Versions of packages xen-hypervisor-4.5-amd64 recommends: > >> ii xen-utils-4.5 4.5.0-1 > >> > >> xen-hypervisor-4.5-amd64 suggests no packages. > >> > >> -- no debconf information > >> > >> ___ > >> Pkg-xen-devel mailing list > >> pkg-xen-de...@lists.alioth.debian.org > >> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-xen-devel > > > > > > ___ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > http://lists.xen.org/xen-devel > > > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [ovmf test] 55883: regressions - FAIL
On Thu, 2015-05-14 at 01:19 +, osstest service user wrote: > flight 55883 ovmf real [real] > http://logs.test-lab.xenproject.org/osstest/logs/55883/ > > Regressions :-( > > Tests which did not succeed and are blocking, > including tests which could not be run: > build-i386-xsm5 xen-build fail REGR. vs. > 55353 > build-amd64 5 xen-build fail REGR. vs. > 55353 > build-i3865 xen-build fail REGR. vs. > 55353 > build-amd64-xsm 5 xen-build fail REGR. vs. > 55353 These seem to be a real upstream build failure, e.g. http://logs.test-lab.xenproject.org/osstest/logs/55883/build-amd64/5.ts-xen-build.log: [...]/MdePkg/Include/Library/BaseLib.h:1560:1: note: expected 'const CHAR8 *' but argument is of type 'UINT8 *' [...]/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c: In function 'ConstructConfigHdr': [...]/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c:1677:15: error: pointer targets in assignment differ in signedness [-Werror=pointer-sign] [...]/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c:1687:15: error: pointer targets in assignment differ in signedness [-Werror=pointer-sign] [...]/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c: In function 'EnumerateAllKeywords': [...]/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c:2636:11: error: passing argument 3 of 'ExtractConfigRequest' from incompatible pointer type [-Werror] [...]/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c:1974:1: note: expected 'UINT8 **' but argument is of type 'CHAR8 **' [...]/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c:2664:11: error: pointer targets in passing argument 1 of 'ExtractReadOnlyFromOpCode' differ in signedness [-Werror=pointer-sign] [...]/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c:503:1: note: expected 'UINT8 *' but argument is of type 'CHAR8 *' cc1: all warnings being treated as errors Seems to be fixed in the upstream tree already. So next time should be ok. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 07/16] libxc/save: introduce setup() and cleanup() on save
introduce setup() and cleanup() which subsume the ctx->save.ops.{setup,cleanup}() calls. The SHADOW_OP_OFF hypercall also included in the cleanup(). Signed-off-by: Yang Hongyang CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper Reviewed-by: Andrew Cooper --- tools/libxc/xc_sr_save.c | 34 +++--- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index caa727d..f4ab5c5 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -637,6 +637,31 @@ static int send_domain_memory_nonlive(struct xc_sr_context *ctx) return rc; } +static int setup(struct xc_sr_context *ctx) +{ +int rc; + +rc = ctx->save.ops.setup(ctx); +if ( rc ) +goto err; + +rc = 0; + + err: +return rc; +} + +static void cleanup(struct xc_sr_context *ctx) +{ +xc_interface *xch = ctx->xch; + +xc_shadow_control(xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_OFF, + NULL, 0, NULL, 0, NULL); + +if ( ctx->save.ops.cleanup(ctx) ) +PERROR("Failed to clean up"); +} + /* * Save a domain. */ @@ -648,7 +673,7 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type) IPRINTF("Saving domain %d, type %s", ctx->domid, dhdr_type_to_str(guest_type)); -rc = ctx->save.ops.setup(ctx); +rc = setup(ctx); if ( rc ) goto err; @@ -701,12 +726,7 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type) PERROR("Save failed"); done: -xc_shadow_control(xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_OFF, - NULL, 0, NULL, 0, NULL); - -rc = ctx->save.ops.cleanup(ctx); -if ( rc ) -PERROR("Failed to clean up"); +cleanup(ctx); if ( saved_rc ) { -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 02/16] libxc/save: Adjust stream-position callbacks for checkpointed streams
From: Andrew Cooper There are some records which should only be sent once in the stream, and not repeated for each checkpoint. {start,end}_of_stream() become per-checkpoint, and a new start_of_stream() is introduced. There is no resulting change record order, but the X86_PV_INFO record is identified as once per stream. Currently the X86_PV_P2M_FRAMES record is as well, but this is because of an implementation bug and can move back to being on an as-needed basis when fixed. In addition, a few minor adjustments of comments and layout. Signed-off-by: Andrew Cooper CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Yang Hongyang --- tools/libxc/xc_sr_common.h | 21 + tools/libxc/xc_sr_save.c | 10 +++--- tools/libxc/xc_sr_save_x86_hvm.c | 23 +++ tools/libxc/xc_sr_save_x86_pv.c | 29 +++-- 4 files changed, 54 insertions(+), 29 deletions(-) diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h index ef42412..c4fe92c 100644 --- a/tools/libxc/xc_sr_common.h +++ b/tools/libxc/xc_sr_common.h @@ -61,19 +61,24 @@ struct xc_sr_save_ops int (*setup)(struct xc_sr_context *ctx); /** - * Write records which need to be at the start of the stream. This is - * called after the Image and Domain headers are written. (Any records - * which need to be ahead of the memory.) + * Send records which need to be at the start of the stream. This is + * called once, after the Image and Domain headers are written. */ int (*start_of_stream)(struct xc_sr_context *ctx); /** - * Write records which need to be at the end of the stream, following the - * complete memory contents. The caller shall handle writing the END - * record into the stream. (Any records which need to be after the memory - * is complete.) + * Send records which need to be at the start of a checkpoint. This is + * called once, or once per checkpoint in a checkpointed stream, and is + * ahead of memory data. */ -int (*end_of_stream)(struct xc_sr_context *ctx); +int (*start_of_checkpoint)(struct xc_sr_context *ctx); + +/** + * Send records which need to be at the end of the checkpoint. This is + * called once, or once per checkpoint in a checkpointed stream, and is + * after the memory data. + */ +int (*end_of_checkpoint)(struct xc_sr_context *ctx); /** * Clean up the local environment. Will be called exactly once, either diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index 83f0591..66fcd3e 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -662,6 +662,10 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type) if ( rc ) goto err; +rc = ctx->save.ops.start_of_checkpoint(ctx); +if ( rc ) +goto err; + if ( ctx->save.live ) rc = send_domain_memory_live(ctx); else @@ -678,12 +682,12 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type) goto err; } -xc_report_progress_single(xch, "End of stream"); - -rc = ctx->save.ops.end_of_stream(ctx); +rc = ctx->save.ops.end_of_checkpoint(ctx); if ( rc ) goto err; +xc_report_progress_single(xch, "End of stream"); + rc = write_end_record(ctx); if ( rc ) goto err; diff --git a/tools/libxc/xc_sr_save_x86_hvm.c b/tools/libxc/xc_sr_save_x86_hvm.c index 58efdb9..f4604db 100644 --- a/tools/libxc/xc_sr_save_x86_hvm.c +++ b/tools/libxc/xc_sr_save_x86_hvm.c @@ -184,7 +184,13 @@ static int x86_hvm_start_of_stream(struct xc_sr_context *ctx) return 0; } -static int x86_hvm_end_of_stream(struct xc_sr_context *ctx) +static int x86_hvm_start_of_checkpoint(struct xc_sr_context *ctx) +{ +/* no-op */ +return 0; +} + +static int x86_hvm_end_of_checkpoint(struct xc_sr_context *ctx) { int rc; @@ -209,7 +215,7 @@ static int x86_hvm_end_of_stream(struct xc_sr_context *ctx) if ( rc ) return rc; -return rc; +return 0; } static int x86_hvm_cleanup(struct xc_sr_context *ctx) @@ -230,12 +236,13 @@ static int x86_hvm_cleanup(struct xc_sr_context *ctx) struct xc_sr_save_ops save_ops_x86_hvm = { -.pfn_to_gfn = x86_hvm_pfn_to_gfn, -.normalise_page = x86_hvm_normalise_page, -.setup = x86_hvm_setup, -.start_of_stream = x86_hvm_start_of_stream, -.end_of_stream = x86_hvm_end_of_stream, -.cleanup = x86_hvm_cleanup, +.pfn_to_gfn = x86_hvm_pfn_to_gfn, +.normalise_page = x86_hvm_normalise_page, +.setup = x86_hvm_setup, +.start_of_stream = x86_hvm_start_of_stream, +.start_of_checkpoint = x86_hvm_start_of_checkpoint, +.end_of_checkpoint = x86_hvm_end_of_checkpoint, +.cleanup = x86_hvm_cleanup, }; /* diff --git a/tools/libxc/xc_sr_save_x86_pv.c b/tools/libxc/xc_sr_save_x86_pv.c index
[Xen-devel] [PATCH v6 10/16] libxc/save: remove bitmap param from send_some_pages
In last patch we added dirty bitmap to the save context, we no longer need to pass this param to send_some_pages. We can get dirty bitmap from the save context. 'entries' should stay as it is a useful sanity check. Signed-off-by: Yang Hongyang CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper Reviewed-by: Andrew Cooper --- tools/libxc/xc_sr_save.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index beb54c4..adb5cce 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -374,23 +374,24 @@ static int send_all_pages(struct xc_sr_context *ctx) } /* - * Send a subset of pages in the guests p2m, according to the provided bitmap. + * Send a subset of pages in the guests p2m, according to the dirty bitmap. * Used for each subsequent iteration of the live migration loop. * * Bitmap is bounded by p2m_size. */ static int send_some_pages(struct xc_sr_context *ctx, - unsigned long *bitmap, unsigned long entries) { xc_interface *xch = ctx->xch; xen_pfn_t p; unsigned long written; int rc; +DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap, +&ctx->save.dirty_bitmap_hbuf); for ( p = 0, written = 0; p < ctx->save.p2m_size; ++p ) { -if ( !test_bit(p, bitmap) ) +if ( !test_bit(p, dirty_bitmap) ) continue; rc = add_to_batch(ctx, p); @@ -515,7 +516,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) if ( rc ) goto out; -rc = send_some_pages(ctx, dirty_bitmap, stats.dirty_count); +rc = send_some_pages(ctx, stats.dirty_count); if ( rc ) goto out; } @@ -540,8 +541,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) bitmap_or(dirty_bitmap, ctx->save.deferred_pages, ctx->save.p2m_size); -rc = send_some_pages(ctx, dirty_bitmap, - stats.dirty_count + ctx->save.nr_deferred_pages); +rc = send_some_pages(ctx, stats.dirty_count + ctx->save.nr_deferred_pages); if ( rc ) goto out; -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 08/16] libxc/save: rename to_send to dirty_bitmap
rename to_send to dirty_bitmap. Signed-off-by: Yang Hongyang CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper Reviewed-by: Andrew Cooper --- tools/libxc/xc_sr_save.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index f4ab5c5..5d08620 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -475,20 +475,20 @@ static int update_progress_string(struct xc_sr_context *ctx, static int send_domain_memory_live(struct xc_sr_context *ctx) { xc_interface *xch = ctx->xch; -DECLARE_HYPERCALL_BUFFER(unsigned long, to_send); +DECLARE_HYPERCALL_BUFFER(unsigned long, dirty_bitmap); xc_shadow_op_stats_t stats = { 0, ctx->save.p2m_size }; char *progress_str = NULL; unsigned x; int rc = -1; -to_send = xc_hypercall_buffer_alloc_pages( -xch, to_send, NRPAGES(bitmap_size(ctx->save.p2m_size))); +dirty_bitmap = xc_hypercall_buffer_alloc_pages( +xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->save.p2m_size))); ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE * sizeof(*ctx->save.batch_pfns)); ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size)); -if ( !ctx->save.batch_pfns || !to_send || !ctx->save.deferred_pages ) +if ( !ctx->save.batch_pfns || !dirty_bitmap || !ctx->save.deferred_pages ) { ERROR("Unable to allocate memory for to_{send,fix}/batch bitmaps"); goto out; @@ -512,7 +512,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) { if ( xc_shadow_control( xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_CLEAN, - HYPERCALL_BUFFER(to_send), ctx->save.p2m_size, + HYPERCALL_BUFFER(dirty_bitmap), ctx->save.p2m_size, NULL, 0, &stats) != ctx->save.p2m_size ) { PERROR("Failed to retrieve logdirty bitmap"); @@ -527,7 +527,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) if ( rc ) goto out; -rc = send_some_pages(ctx, to_send, stats.dirty_count); +rc = send_some_pages(ctx, dirty_bitmap, stats.dirty_count); if ( rc ) goto out; } @@ -538,7 +538,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) if ( xc_shadow_control( xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_CLEAN, - HYPERCALL_BUFFER(to_send), ctx->save.p2m_size, + HYPERCALL_BUFFER(dirty_bitmap), ctx->save.p2m_size, NULL, 0, &stats) != ctx->save.p2m_size ) { PERROR("Failed to retrieve logdirty bitmap"); @@ -550,9 +550,9 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) if ( rc ) goto out; -bitmap_or(to_send, ctx->save.deferred_pages, ctx->save.p2m_size); +bitmap_or(dirty_bitmap, ctx->save.deferred_pages, ctx->save.p2m_size); -rc = send_some_pages(ctx, to_send, +rc = send_some_pages(ctx, dirty_bitmap, stats.dirty_count + ctx->save.nr_deferred_pages); if ( rc ) goto out; @@ -578,7 +578,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) if ( xc_shadow_control( xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_PEEK, - HYPERCALL_BUFFER(to_send), ctx->save.p2m_size, + HYPERCALL_BUFFER(dirty_bitmap), ctx->save.p2m_size, NULL, 0, &stats) != ctx->save.p2m_size ) { PERROR("Failed to retrieve logdirty bitmap"); @@ -593,7 +593,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) out: xc_set_progress_prefix(xch, NULL); free(progress_str); -xc_hypercall_buffer_free_pages(xch, to_send, +xc_hypercall_buffer_free_pages(xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->save.p2m_size))); free(ctx->save.deferred_pages); free(ctx->save.batch_pfns); -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 05/16] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW
There are cases where we only need to use the hypercall buffer data, and do not use the xc_hypercall_buffer_t struct. DECLARE_HYPERCALL_BUFFER_SHADOW defines a user pointer that can allow us to access the hypercall buffer data but it also defines a xc_hypercall_buffer_t that we don't use, the compiler will report arg unused error. Add __attribute__((unused)) before xc_hypercall_buffer_t to avoid the compiler error. Example cases: In send_all_pages(), we only need to use the hypercall buffer data which is a dirty bitmap, we set the dirty bitmap to all dirty and call send_dirty_pages, we will not use the xc_hypercall_buffer_t and hypercall to retrieve the dirty bitmap. In send_some_pages(), we will also only need to use the dirty_bitmap. the retrieve dirty bitmap hypercall are done by the caller. Signed-off-by: Yang Hongyang CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper --- tools/libxc/include/xenctrl.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index a689caf..43fc31c 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -289,6 +289,7 @@ typedef struct xc_hypercall_buffer xc_hypercall_buffer_t; */ #define DECLARE_HYPERCALL_BUFFER_SHADOW(_type, _name, _hbuf) \ _type *(_name) = (_hbuf)->hbuf;\ +__attribute__((unused))\ xc_hypercall_buffer_t XC__HYPERCALL_BUFFER_NAME(_name) = { \ .hbuf = (void *)-1,\ .param_shadow = (_hbuf), \ -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 04/16] libxc/migration: Pass checkpoint information into the save algorithm.
From: Andrew Cooper Signed-off-by: Andrew Cooper CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Yang Hongyang --- tools/libxc/include/xenguest.h | 1 + tools/libxc/xc_sr_common.h | 3 +++ tools/libxc/xc_sr_save.c | 3 +++ tools/libxl/libxl_dom.c| 1 + 4 files changed, 8 insertions(+) diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h index 8e39075..7581263 100644 --- a/tools/libxc/include/xenguest.h +++ b/tools/libxc/include/xenguest.h @@ -30,6 +30,7 @@ #define XCFLAGS_HVM (1 << 2) #define XCFLAGS_STDVGA(1 << 3) #define XCFLAGS_CHECKPOINT_COMPRESS(1 << 4) +#define XCFLAGS_CHECKPOINTED(1 << 5) #define X86_64_B_SIZE 64 #define X86_32_B_SIZE 32 diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h index c4fe92c..c0f90d4 100644 --- a/tools/libxc/xc_sr_common.h +++ b/tools/libxc/xc_sr_common.h @@ -174,6 +174,9 @@ struct xc_sr_context /* Live migrate vs non live suspend. */ bool live; +/* Plain VM, or checkpoints over time. */ +bool checkpointed; + /* Further debugging information in the stream. */ bool debug; diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index 66fcd3e..caa727d 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -732,6 +732,7 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom, ctx.save.callbacks = callbacks; ctx.save.live = !!(flags & XCFLAGS_LIVE); ctx.save.debug = !!(flags & XCFLAGS_DEBUG); +ctx.save.checkpointed = !!(flags & XCFLAGS_CHECKPOINTED); /* * TODO: Find some time to better tweak the live migration algorithm. @@ -745,6 +746,8 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom, /* Sanity checks for callbacks. */ if ( hvm ) assert(callbacks->switch_qemu_logdirty); +if ( ctx.save.checkpointed ) +assert(callbacks->checkpoint && callbacks->postcopy); IPRINTF("In experimental %s", __func__); DPRINTF("fd %d, dom %u, max_iters %u, max_factor %u, flags %u, hvm %d", diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index f408646..a0c9850 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -2003,6 +2003,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss) if (r_info != NULL) { dss->interval = r_info->interval; +dss->xcflags |= XCFLAGS_CHECKPOINTED; if (libxl_defbool_val(r_info->compression)) dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS; } -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 15/16] libxc/restore: introduce setup() and cleanup() on restore
introduce setup() and cleanup() which subsume the ctx->restore.ops.{setup,cleanup}() calls and also do memory alloc/free. Signed-off-by: Yang Hongyang CC: Andrew Cooper CC: Ian Campbell CC: Ian Jackson CC: Wei Liu --- tools/libxc/xc_sr_restore.c | 48 - 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c index 8022c3d..2345f66 100644 --- a/tools/libxc/xc_sr_restore.c +++ b/tools/libxc/xc_sr_restore.c @@ -510,6 +510,38 @@ static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec) return rc; } +static int setup(struct xc_sr_context *ctx) +{ +xc_interface *xch = ctx->xch; +int rc; + +rc = ctx->restore.ops.setup(ctx); +if ( rc ) +goto err; + +ctx->restore.max_populated_pfn = (32 * 1024 / 4) - 1; +ctx->restore.populated_pfns = bitmap_alloc( +ctx->restore.max_populated_pfn + 1); +if ( !ctx->restore.populated_pfns ) +{ +ERROR("Unable to allocate memory for populated_pfns bitmap"); +rc = -1; +goto err; +} + + err: +return rc; +} + +static void cleanup(struct xc_sr_context *ctx) +{ +xc_interface *xch = ctx->xch; + +free(ctx->restore.populated_pfns); +if ( ctx->restore.ops.cleanup(ctx) ) +PERROR("Failed to clean up"); +} + #ifdef XG_LIBXL_HVM_COMPAT extern int read_qemu(struct xc_sr_context *ctx); #endif @@ -524,19 +556,10 @@ static int restore(struct xc_sr_context *ctx) IPRINTF("Restoring domain"); -rc = ctx->restore.ops.setup(ctx); +rc = setup(ctx); if ( rc ) goto err; -ctx->restore.max_populated_pfn = (32 * 1024 / 4) - 1; -ctx->restore.populated_pfns = bitmap_alloc( -ctx->restore.max_populated_pfn + 1); -if ( !ctx->restore.populated_pfns ) -{ -ERROR("Unable to allocate memory for populated_pfns bitmap"); -goto err; -} - do { rc = read_record(ctx, &rec); @@ -571,10 +594,7 @@ static int restore(struct xc_sr_context *ctx) PERROR("Restore failed"); done: -free(ctx->restore.populated_pfns); -rc = ctx->restore.ops.cleanup(ctx); -if ( rc ) -PERROR("Failed to clean up"); +cleanup(ctx); if ( saved_rc ) { -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 09/16] libxc/save: adjust the memory allocation for migration
Move the memory allocation before the concrete live/nolive save in order to avoid the free/alloc memory loop when using Remus. Signed-off-by: Yang Hongyang CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper Reviewed-by: Andrew Cooper --- tools/libxc/xc_sr_common.h | 1 + tools/libxc/xc_sr_save.c | 60 ++ 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h index c0f90d4..276d00a 100644 --- a/tools/libxc/xc_sr_common.h +++ b/tools/libxc/xc_sr_common.h @@ -190,6 +190,7 @@ struct xc_sr_context unsigned nr_batch_pfns; unsigned long *deferred_pages; unsigned long nr_deferred_pages; +xc_hypercall_buffer_t dirty_bitmap_hbuf; } save; struct /* Restore data. */ diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index 5d08620..beb54c4 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -475,24 +475,12 @@ static int update_progress_string(struct xc_sr_context *ctx, static int send_domain_memory_live(struct xc_sr_context *ctx) { xc_interface *xch = ctx->xch; -DECLARE_HYPERCALL_BUFFER(unsigned long, dirty_bitmap); xc_shadow_op_stats_t stats = { 0, ctx->save.p2m_size }; char *progress_str = NULL; unsigned x; int rc = -1; - -dirty_bitmap = xc_hypercall_buffer_alloc_pages( -xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->save.p2m_size))); - -ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE * - sizeof(*ctx->save.batch_pfns)); -ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size)); - -if ( !ctx->save.batch_pfns || !dirty_bitmap || !ctx->save.deferred_pages ) -{ -ERROR("Unable to allocate memory for to_{send,fix}/batch bitmaps"); -goto out; -} +DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap, +&ctx->save.dirty_bitmap_hbuf); rc = enable_logdirty(ctx); if ( rc ) @@ -593,10 +581,6 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) out: xc_set_progress_prefix(xch, NULL); free(progress_str); -xc_hypercall_buffer_free_pages(xch, dirty_bitmap, - NRPAGES(bitmap_size(ctx->save.p2m_size))); -free(ctx->save.deferred_pages); -free(ctx->save.batch_pfns); return rc; } @@ -609,17 +593,6 @@ static int send_domain_memory_nonlive(struct xc_sr_context *ctx) xc_interface *xch = ctx->xch; int rc = -1; -ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE * - sizeof(*ctx->save.batch_pfns)); -ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size)); - -if ( !ctx->save.batch_pfns || !ctx->save.deferred_pages ) -{ -PERROR("Failed to allocate memory for nonlive tracking structures"); -errno = ENOMEM; -goto err; -} - rc = suspend_domain(ctx); if ( rc ) goto err; @@ -631,15 +604,30 @@ static int send_domain_memory_nonlive(struct xc_sr_context *ctx) goto err; err: -free(ctx->save.deferred_pages); -free(ctx->save.batch_pfns); - return rc; } static int setup(struct xc_sr_context *ctx) { +xc_interface *xch = ctx->xch; int rc; +DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap, +&ctx->save.dirty_bitmap_hbuf); + +dirty_bitmap = xc_hypercall_buffer_alloc_pages( + xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->save.p2m_size))); +ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE * + sizeof(*ctx->save.batch_pfns)); +ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size)); + +if ( !ctx->save.batch_pfns || !dirty_bitmap || !ctx->save.deferred_pages ) +{ +ERROR("Unable to allocate memory for dirty bitmaps, batch pfns and" + " deferred pages"); +rc = -1; +errno = ENOMEM; +goto err; +} rc = ctx->save.ops.setup(ctx); if ( rc ) @@ -654,12 +642,20 @@ static int setup(struct xc_sr_context *ctx) static void cleanup(struct xc_sr_context *ctx) { xc_interface *xch = ctx->xch; +DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap, +&ctx->save.dirty_bitmap_hbuf); + xc_shadow_control(xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_OFF, NULL, 0, NULL, 0, NULL); if ( ctx->save.ops.cleanup(ctx) ) PERROR("Failed to clean up"); + +xc_hypercall_buffer_free_pages(xch, dirty_bitmap, + NRPAGES(bitmap_size(ctx->save.p2m_size))); +free(ctx->save.deferred_pages); +free(ctx->save.batch_pfns); } /* -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen
[Xen-devel] [PATCH v6 13/16] libxc/restore: introduce process_record()
Move record handle codes into a function process_record(). It will be used multiple times by Remus. No functional change. Signed-off-by: Yang Hongyang CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper Reviewed-by: Andrew Cooper --- tools/libxc/xc_sr_restore.c | 77 + 1 file changed, 43 insertions(+), 34 deletions(-) diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c index 0bf4bae..53bd674 100644 --- a/tools/libxc/xc_sr_restore.c +++ b/tools/libxc/xc_sr_restore.c @@ -468,6 +468,48 @@ static int handle_page_data(struct xc_sr_context *ctx, struct xc_sr_record *rec) return rc; } +static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec) +{ +xc_interface *xch = ctx->xch; +int rc = 0; + +switch ( rec->type ) +{ +case REC_TYPE_END: +break; + +case REC_TYPE_PAGE_DATA: +rc = handle_page_data(ctx, rec); +break; + +case REC_TYPE_VERIFY: +DPRINTF("Verify mode enabled"); +ctx->restore.verify = true; +break; + +default: +rc = ctx->restore.ops.process_record(ctx, rec); +break; +} + +free(rec->data); + +if ( rc == RECORD_NOT_PROCESSED ) +{ +if ( rec->type & REC_TYPE_OPTIONAL ) +DPRINTF("Ignoring optional record %#x (%s)", +rec->type, rec_type_to_str(rec->type)); +else +{ +ERROR("Mandatory record %#x (%s) not handled", + rec->type, rec_type_to_str(rec->type)); +rc = -1; +} +} + +return rc; +} + /* * Restore a domain. */ @@ -498,40 +540,7 @@ static int restore(struct xc_sr_context *ctx) if ( rc ) goto err; -switch ( rec.type ) -{ -case REC_TYPE_END: -break; - -case REC_TYPE_PAGE_DATA: -rc = handle_page_data(ctx, &rec); -break; - -case REC_TYPE_VERIFY: -DPRINTF("Verify mode enabled"); -ctx->restore.verify = true; -break; - -default: -rc = ctx->restore.ops.process_record(ctx, &rec); -break; -} - -free(rec.data); - -if ( rc == RECORD_NOT_PROCESSED ) -{ -if ( rec.type & REC_TYPE_OPTIONAL ) -DPRINTF("Ignoring optional record %#x (%s)", -rec.type, rec_type_to_str(rec.type)); -else -{ -ERROR("Mandatory record %#x (%s) not handled", - rec.type, rec_type_to_str(rec.type)); -rc = -1; -} -} - +rc = process_record(ctx, &rec); if ( rc ) goto err; -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 06/16] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro
When we use a DECLARE_HYPERCALL_BUFFER_SHADOW define a user pointer '_name' and a shadow xc_hypercall_buffer_t. then call xc_hypercall_buffer_free_pages(_xch, _name, _nr), the complier will report '_name' unused error, it's because xc_hypercall_buffer_free_pages() is a MACRO and '_name' transparently converted to the hypercall buffer. it confused the caller because xc_hypercall_buffer_free_pages() do look like a function and take '_name' as an arg. Add an if check to let the compiler think we are actually using the argument '_name'. Signed-off-by: Yang Hongyang CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper --- tools/libxc/include/xenctrl.h | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 43fc31c..448063c 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -350,7 +350,12 @@ void xc__hypercall_buffer_free(xc_interface *xch, xc_hypercall_buffer_t *b); void *xc__hypercall_buffer_alloc_pages(xc_interface *xch, xc_hypercall_buffer_t *b, int nr_pages); #define xc_hypercall_buffer_alloc_pages(_xch, _name, _nr) xc__hypercall_buffer_alloc_pages(_xch, HYPERCALL_BUFFER(_name), _nr) void xc__hypercall_buffer_free_pages(xc_interface *xch, xc_hypercall_buffer_t *b, int nr_pages); -#define xc_hypercall_buffer_free_pages(_xch, _name, _nr) xc__hypercall_buffer_free_pages(_xch, HYPERCALL_BUFFER(_name), _nr) +#define xc_hypercall_buffer_free_pages(_xch, _name, _nr)\ +do {\ +if ( _name )\ +xc__hypercall_buffer_free_pages(_xch, HYPERCALL_BUFFER(_name), \ +_nr); \ +} while (0) /* * Array of hypercall buffers. -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 14/16] libxc/restore: split read/handle qemu info
Split read/handle qemu info. The receiving of qemu info should be done while we receive the migration stream, handle_qemu will be called when the stream complete. Otherwise, it will break Remus because read_record() won't read qemu info and stream_complete will be called at failover. Signed-off-by: Yang Hongyang CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper --- tools/libxc/xc_sr_common.h | 5 + tools/libxc/xc_sr_restore.c | 12 tools/libxc/xc_sr_restore_x86_hvm.c | 28 +--- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h index 276d00a..0ba9728 100644 --- a/tools/libxc/xc_sr_common.h +++ b/tools/libxc/xc_sr_common.h @@ -288,6 +288,11 @@ struct xc_sr_context /* HVM context blob. */ void *context; size_t contextsz; + +#ifdef XG_LIBXL_HVM_COMPAT +uint32_t qlen; +void *qbuf; +#endif } restore; }; } x86_hvm; diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c index 53bd674..8022c3d 100644 --- a/tools/libxc/xc_sr_restore.c +++ b/tools/libxc/xc_sr_restore.c @@ -510,6 +510,9 @@ static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec) return rc; } +#ifdef XG_LIBXL_HVM_COMPAT +extern int read_qemu(struct xc_sr_context *ctx); +#endif /* * Restore a domain. */ @@ -546,6 +549,15 @@ static int restore(struct xc_sr_context *ctx) } while ( rec.type != REC_TYPE_END ); +#ifdef XG_LIBXL_HVM_COMPAT +if ( ctx->dominfo.hvm ) +{ +rc = read_qemu(ctx); +if ( rc ) +goto err; +} +#endif + rc = ctx->restore.ops.stream_complete(ctx); if ( rc ) goto err; diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c b/tools/libxc/xc_sr_restore_x86_hvm.c index 6e9b318..6f5af0e 100644 --- a/tools/libxc/xc_sr_restore_x86_hvm.c +++ b/tools/libxc/xc_sr_restore_x86_hvm.c @@ -94,14 +94,14 @@ static int handle_hvm_params(struct xc_sr_context *ctx, } #ifdef XG_LIBXL_HVM_COMPAT -static int handle_qemu(struct xc_sr_context *ctx) +int read_qemu(struct xc_sr_context *ctx); +int read_qemu(struct xc_sr_context *ctx) { xc_interface *xch = ctx->xch; -char qemusig[21], path[256]; +char qemusig[21]; uint32_t qlen; void *qbuf = NULL; int rc = -1; -FILE *fp = NULL; if ( read_exact(ctx->fd, qemusig, sizeof(qemusig)) ) { @@ -137,6 +137,28 @@ static int handle_qemu(struct xc_sr_context *ctx) goto out; } +/* With Remus, this could be read many times */ +if ( ctx->x86_hvm.restore.qbuf ) +free(ctx->x86_hvm.restore.qbuf); +ctx->x86_hvm.restore.qbuf = qbuf; +ctx->x86_hvm.restore.qlen = qlen; +rc = 0; + +out: +if (rc) +free(qbuf); +return rc; +} + +static int handle_qemu(struct xc_sr_context *ctx) +{ +xc_interface *xch = ctx->xch; +char path[256]; +uint32_t qlen = ctx->x86_hvm.restore.qlen; +void *qbuf = ctx->x86_hvm.restore.qbuf; +int rc = -1; +FILE *fp = NULL; + sprintf(path, XC_DEVICE_MODEL_RESTORE_FILE".%u", ctx->domid); fp = fopen(path, "wb"); if ( !fp ) -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 11/16] libxc/save: rename send_some_pages to send_dirty_pages
rename send_some_pages to send_dirty_pages, no functional change. Signed-off-by: Yang Hongyang CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper --- tools/libxc/xc_sr_save.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index adb5cce..3768bda 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -379,8 +379,8 @@ static int send_all_pages(struct xc_sr_context *ctx) * * Bitmap is bounded by p2m_size. */ -static int send_some_pages(struct xc_sr_context *ctx, - unsigned long entries) +static int send_dirty_pages(struct xc_sr_context *ctx, +unsigned long entries) { xc_interface *xch = ctx->xch; xen_pfn_t p; @@ -516,7 +516,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) if ( rc ) goto out; -rc = send_some_pages(ctx, stats.dirty_count); +rc = send_dirty_pages(ctx, stats.dirty_count); if ( rc ) goto out; } @@ -541,7 +541,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) bitmap_or(dirty_bitmap, ctx->save.deferred_pages, ctx->save.p2m_size); -rc = send_some_pages(ctx, stats.dirty_count + ctx->save.nr_deferred_pages); +rc = send_dirty_pages(ctx, stats.dirty_count + ctx->save.nr_deferred_pages); if ( rc ) goto out; -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 12/16] libxc/save: reuse send_dirty_pages() in send_all_pages()
introduce bitmap_set() to set the entire bitmap. in send_all_pages(), set the entire bitmap and call send_dirty_pages(). Signed-off-by: Yang Hongyang CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper Reviewed-by: Andrew Cooper --- tools/libxc/xc_bitops.h | 5 + tools/libxc/xc_sr_save.c | 44 ++-- 2 files changed, 19 insertions(+), 30 deletions(-) diff --git a/tools/libxc/xc_bitops.h b/tools/libxc/xc_bitops.h index dfce3b8..cd749f4 100644 --- a/tools/libxc/xc_bitops.h +++ b/tools/libxc/xc_bitops.h @@ -26,6 +26,11 @@ static inline unsigned long *bitmap_alloc(int nr_bits) return calloc(1, bitmap_size(nr_bits)); } +static inline void bitmap_set(unsigned long *addr, int nr_bits) +{ +memset(addr, 0xff, bitmap_size(nr_bits)); +} + static inline void bitmap_clear(unsigned long *addr, int nr_bits) { memset(addr, 0, bitmap_size(nr_bits)); diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index 3768bda..1d0a46d 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -344,36 +344,6 @@ static int suspend_domain(struct xc_sr_context *ctx) } /* - * Send all pages in the guests p2m. Used as the first iteration of the live - * migration loop, and for a non-live save. - */ -static int send_all_pages(struct xc_sr_context *ctx) -{ -xc_interface *xch = ctx->xch; -xen_pfn_t p; -int rc; - -for ( p = 0; p < ctx->save.p2m_size; ++p ) -{ -rc = add_to_batch(ctx, p); -if ( rc ) -return rc; - -/* Update progress every 4MB worth of memory sent. */ -if ( (p & ((1U << (22 - 12)) - 1)) == 0 ) -xc_report_progress_step(xch, p, ctx->save.p2m_size); -} - -rc = flush_batch(ctx); -if ( rc ) -return rc; - -xc_report_progress_step(xch, ctx->save.p2m_size, -ctx->save.p2m_size); -return 0; -} - -/* * Send a subset of pages in the guests p2m, according to the dirty bitmap. * Used for each subsequent iteration of the live migration loop. * @@ -416,6 +386,20 @@ static int send_dirty_pages(struct xc_sr_context *ctx, return 0; } +/* + * Send all pages in the guests p2m. Used as the first iteration of the live + * migration loop, and for a non-live save. + */ +static int send_all_pages(struct xc_sr_context *ctx) +{ +DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap, +&ctx->save.dirty_bitmap_hbuf); + +bitmap_set(dirty_bitmap, ctx->save.p2m_size); + +return send_dirty_pages(ctx, ctx->save.p2m_size); +} + static int enable_logdirty(struct xc_sr_context *ctx) { xc_interface *xch = ctx->xch; -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 00/16] Misc patches to aid migration v2 Remus support
This is the combination of Andrew Cooper's misc patches and mine to aid migration v2 Remus support. See individual patches for details. Git tree available at: https://github.com/macrosheep/xen/tree/misc-remus-v6 v5->v6 Some minor changes, typos etc. v4->v5 Most of the changes are trival, like drop brackets in DECLARE_HYPERCALL_BUFFER_SHADOW(), drop stray blank line etc. Except the 6th patch which add a do{}while (0) in xc_hypercall_buffer_free_pages macro. Summary of changes: M = modified A = acked N = new no mark = unchanged from last round Andrew Cooper (4): A libxc/migration: Be rather stricter with illformed callers libxc/save: Adjust stream-position callbacks for checkpointed streams A libxc/migration: Specification update for CHECKPOINT records libxc/migration: Pass checkpoint information into the save algorithm. Yang Hongyang (12): M tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW tools/libxc: add a check in xc_hypercall_buffer_free_pages macro libxc/save: introduce setup() and cleanup() on save libxc/save: rename to_send to dirty_bitmap libxc/save: adjust the memory allocation for migration libxc/save: remove bitmap param from send_some_pages libxc/save: rename send_some_pages to send_dirty_pages libxc/save: reuse send_dirty_pages() in send_all_pages() libxc/restore: introduce process_record() libxc/restore: split read/handle qemu info N libxc/restore: introduce setup() and cleanup() on restore N libxc/restore: add checkpointed flag to the restore context docs/specs/libxc-migration-stream.pandoc | 29 +- tools/libxc/include/xenctrl.h| 8 +- tools/libxc/include/xenguest.h | 1 + tools/libxc/xc_bitops.h | 5 + tools/libxc/xc_sr_common.c | 1 + tools/libxc/xc_sr_common.h | 33 -- tools/libxc/xc_sr_restore.c | 130 +++ tools/libxc/xc_sr_restore_x86_hvm.c | 28 - tools/libxc/xc_sr_save.c | 171 --- tools/libxc/xc_sr_save_x86_hvm.c | 30 +++--- tools/libxc/xc_sr_save_x86_pv.c | 29 -- tools/libxc/xc_sr_stream_format.h| 1 + tools/libxl/libxl_dom.c | 1 + 13 files changed, 303 insertions(+), 164 deletions(-) -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 16/16] libxc/restore: add checkpointed flag to the restore context
add checkpointed flag to the restore context. Signed-off-by: Yang Hongyang CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper Reviewed-by: Andrew Cooper --- tools/libxc/xc_sr_common.h | 3 +++ tools/libxc/xc_sr_restore.c | 1 + 2 files changed, 4 insertions(+) diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h index 0ba9728..f8121e7 100644 --- a/tools/libxc/xc_sr_common.h +++ b/tools/libxc/xc_sr_common.h @@ -205,6 +205,9 @@ struct xc_sr_context uint32_t guest_type; uint32_t guest_page_size; +/* Plain VM, or checkpoints over time. */ +bool checkpointed; + /* * Xenstore and Console parameters. * INPUT: evtchn & domid diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c index 2345f66..9ab5760 100644 --- a/tools/libxc/xc_sr_restore.c +++ b/tools/libxc/xc_sr_restore.c @@ -624,6 +624,7 @@ int xc_domain_restore2(xc_interface *xch, int io_fd, uint32_t dom, ctx.restore.console_domid = console_domid; ctx.restore.xenstore_evtchn = store_evtchn; ctx.restore.xenstore_domid = store_domid; +ctx.restore.checkpointed = checkpointed_stream; ctx.restore.callbacks = callbacks; IPRINTF("In experimental %s", __func__); -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 01/16] libxc/migration: Be rather stricter with illformed callers
From: Andrew Cooper The migration code itself should be able to validly assume all mandatory callbacks are set up. Signed-off-by: Andrew Cooper CC: Ian Campbell CC: Ian Jackson CC: Wei Liu CC: Yang Hongyang Acked-by: Ian Campbell --- tools/libxc/xc_sr_save.c | 4 tools/libxc/xc_sr_save_x86_hvm.c | 7 --- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index 5d9c267..83f0591 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -738,6 +738,10 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom, ctx.save.max_iterations = 5; ctx.save.dirty_threshold = 50; +/* Sanity checks for callbacks. */ +if ( hvm ) +assert(callbacks->switch_qemu_logdirty); + IPRINTF("In experimental %s", __func__); DPRINTF("fd %d, dom %u, max_iters %u, max_factor %u, flags %u, hvm %d", io_fd, dom, max_iters, max_factor, flags, hvm); diff --git a/tools/libxc/xc_sr_save_x86_hvm.c b/tools/libxc/xc_sr_save_x86_hvm.c index 8baa104..58efdb9 100644 --- a/tools/libxc/xc_sr_save_x86_hvm.c +++ b/tools/libxc/xc_sr_save_x86_hvm.c @@ -166,13 +166,6 @@ static int x86_hvm_setup(struct xc_sr_context *ctx) { xc_interface *xch = ctx->xch; -if ( !ctx->save.callbacks->switch_qemu_logdirty ) -{ -ERROR("No switch_qemu_logdirty callback provided"); -errno = EINVAL; -return -1; -} - if ( ctx->save.callbacks->switch_qemu_logdirty( ctx->domid, 1, ctx->save.callbacks->data) ) { -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 03/16] libxc/migration: Specification update for CHECKPOINT records
From: Andrew Cooper Checkpointed streams need to signal the end of a consistent view of VM state, and the start of the libxl data. Signed-off-by: Andrew Cooper Signed-off-by: David Vrabel Signed-off-by: Yang Hongyang CC: Ian Campbell CC: Ian Jackson CC: Wei Liu Acked-by: Ian Campbell --- docs/specs/libxc-migration-stream.pandoc | 29 ++--- tools/libxc/xc_sr_common.c | 1 + tools/libxc/xc_sr_stream_format.h| 1 + 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/docs/specs/libxc-migration-stream.pandoc b/docs/specs/libxc-migration-stream.pandoc index 520240f..68fa513 100644 --- a/docs/specs/libxc-migration-stream.pandoc +++ b/docs/specs/libxc-migration-stream.pandoc @@ -37,8 +37,6 @@ Not Yet Included The following features are not yet fully specified and will be included in a future draft. -* Remus - * Page data compression. * ARM @@ -227,7 +225,9 @@ type 0x: END 0x000D: VERIFY - 0x000E - 0x7FFF: Reserved for future _mandatory_ + 0x000E: CHECKPOINT + + 0x000F - 0x7FFF: Reserved for future _mandatory_ records. 0x8000 - 0x: Reserved for future _optional_ @@ -578,6 +578,29 @@ The verify record contains no fields; its body_length is 0. \clearpage +CHECKPOINT +-- + +A checkpoint record indicates that all the preceding records in the stream +represent a consistent view of VM state. + + 0 1 2 3 4 5 6 7 octet ++-+ + +The checkpoint record contains no fields; its body_length is 0 + +If the stream is embedded in a higher level toolstack stream, the +CHECKPOINT record marks the end of the libxc portion of the stream +and the stream is handed back to the higher level for further +processing. + +The higher level stream may then hand the stream back to libxc to +process another set of records for the next consistent VM state +snapshot. This next set of records may be terminated by another +CHECKPOINT record or an END record. + +\clearpage + Layout == diff --git a/tools/libxc/xc_sr_common.c b/tools/libxc/xc_sr_common.c index 59e0c5d..945cfa6 100644 --- a/tools/libxc/xc_sr_common.c +++ b/tools/libxc/xc_sr_common.c @@ -34,6 +34,7 @@ static const char *mandatory_rec_types[] = [REC_TYPE_TOOLSTACK]= "Toolstack", [REC_TYPE_X86_PV_VCPU_MSRS] = "x86 PV vcpu msrs", [REC_TYPE_VERIFY] = "Verify", +[REC_TYPE_CHECKPOINT] = "Checkpoint", }; const char *rec_type_to_str(uint32_t type) diff --git a/tools/libxc/xc_sr_stream_format.h b/tools/libxc/xc_sr_stream_format.h index d116ca6..6d0f8fd 100644 --- a/tools/libxc/xc_sr_stream_format.h +++ b/tools/libxc/xc_sr_stream_format.h @@ -74,6 +74,7 @@ struct xc_sr_rhdr #define REC_TYPE_TOOLSTACK0x000bU #define REC_TYPE_X86_PV_VCPU_MSRS 0x000cU #define REC_TYPE_VERIFY 0x000dU +#define REC_TYPE_CHECKPOINT 0x000eU #define REC_TYPE_OPTIONAL 0x8000U -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH Remus v4 0/2] Remus support for Migration-v2
This patchset implement the Remus support for Migration v2 but without memory compressing. Git tree available at: https://github.com/macrosheep/xen/tree/Remus-newmig-v4 This patchset is based on [PATCH v6 00/16] Misc patches to aid migration v2 Remus support https://github.com/macrosheep/xen/tree/misc-remus-v6 Yang Hongyang (2): libxc/save: implement Remus checkpointed save libxc/restore: implement Remus checkpointed restore tools/libxc/xc_sr_common.h | 14 + tools/libxc/xc_sr_restore.c | 121 tools/libxc/xc_sr_save.c| 80 ++--- 3 files changed, 186 insertions(+), 29 deletions(-) -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH Remus v4 2/2] libxc/restore: implement Remus checkpointed restore
With Remus, the restore flow should be: the first full migration stream -> { periodically restore stream } Signed-off-by: Yang Hongyang Signed-off-by: Andrew Cooper CC: Ian Campbell CC: Ian Jackson CC: Wei Liu --- tools/libxc/xc_sr_common.h | 14 + tools/libxc/xc_sr_restore.c | 121 2 files changed, 125 insertions(+), 10 deletions(-) diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h index f8121e7..3bf27f1 100644 --- a/tools/libxc/xc_sr_common.h +++ b/tools/libxc/xc_sr_common.h @@ -208,6 +208,20 @@ struct xc_sr_context /* Plain VM, or checkpoints over time. */ bool checkpointed; +/* Currently buffering records between a checkpoint */ +bool buffer_all_records; + +/* + * With Remus, we buffer the records sent by the primary at checkpoint, + * in case the primary will fail, we can recover from the last + * checkpoint state. + * This should be enough because primary only send dirty pages at + * checkpoint. + */ +#define MAX_BUF_RECORDS 1024 +struct xc_sr_record *buffered_records; +unsigned buffered_rec_num; + /* * Xenstore and Console parameters. * INPUT: evtchn & domid diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c index 9ab5760..3c93406 100644 --- a/tools/libxc/xc_sr_restore.c +++ b/tools/libxc/xc_sr_restore.c @@ -468,10 +468,73 @@ static int handle_page_data(struct xc_sr_context *ctx, struct xc_sr_record *rec) return rc; } +static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec); +static int handle_checkpoint(struct xc_sr_context *ctx) +{ +xc_interface *xch = ctx->xch; +int rc = 0; +unsigned i; +struct xc_sr_record *rec; + +if ( !ctx->restore.checkpointed ) +{ +ERROR("Found checkpoint in non-checkpointed stream"); +rc = -1; +goto err; +} + +if ( ctx->restore.buffer_all_records ) +{ +IPRINTF("All records buffered"); + +/* + * We need to set buffer_all_records to false in + * order to process records instead of buffer records. + * buffer_all_records should be set back to true after + * we successfully processed all records. + */ +ctx->restore.buffer_all_records = false; +for ( i = 0; i < ctx->restore.buffered_rec_num; i++) +{ +rec = ctx->restore.buffered_records + +i * sizeof(struct xc_sr_record); +rc = process_record(ctx, rec); +if ( rc ) +goto err; +} +ctx->restore.buffered_rec_num = 0; +ctx->restore.buffer_all_records = true; +IPRINTF("All records processed"); +} +else +ctx->restore.buffer_all_records = true; + + err: +return rc; +} + static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec) { xc_interface *xch = ctx->xch; int rc = 0; +struct xc_sr_record *buf_rec; + +if ( ctx->restore.buffer_all_records && + rec->type != REC_TYPE_END && + rec->type != REC_TYPE_CHECKPOINT ) +{ +if ( ctx->restore.buffered_rec_num >= MAX_BUF_RECORDS ) +{ +ERROR("There are too many records within a checkpoint"); +return -1; +} + +buf_rec = ctx->restore.buffered_records + + ctx->restore.buffered_rec_num++ * sizeof(struct xc_sr_record); +memcpy(buf_rec, rec, sizeof(struct xc_sr_record)); + +return 0; +} switch ( rec->type ) { @@ -487,12 +550,17 @@ static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec) ctx->restore.verify = true; break; +case REC_TYPE_CHECKPOINT: +rc = handle_checkpoint(ctx); +break; + default: rc = ctx->restore.ops.process_record(ctx, rec); break; } free(rec->data); +rec->data = NULL; if ( rc == RECORD_NOT_PROCESSED ) { @@ -529,6 +597,15 @@ static int setup(struct xc_sr_context *ctx) goto err; } +ctx->restore.buffered_records = malloc( +MAX_BUF_RECORDS * sizeof(struct xc_sr_record)); +if ( !ctx->restore.buffered_records ) +{ +ERROR("Unable to allocate memory for buffered records"); +rc = -1; +goto err; +} + err: return rc; } @@ -536,7 +613,15 @@ static int setup(struct xc_sr_context *ctx) static void cleanup(struct xc_sr_context *ctx) { xc_interface *xch = ctx->xch; +unsigned i; +struct xc_sr_record *rec; +for ( i = 0; i < ctx->restore.buffered_rec_num; i++) +{ +rec = ctx->restore.buffered_records + i * sizeof(struct xc_sr_record); +free(rec->data); +} +free(ctx->restore.buffered_records); free(ctx->restore.populated_pfns); if ( ctx->restore.ops.cleanup(ctx) ) PERROR("Fai
[Xen-devel] [PATCH Remus v4 1/2] libxc/save: implement Remus checkpointed save
With Remus, the save flow should be: live migration->{ periodically save(checkpointed save) } Signed-off-by: Yang Hongyang Reviewed-by: Andrew Cooper CC: Ian Campbell CC: Ian Jackson CC: Wei Liu --- tools/libxc/xc_sr_save.c | 80 1 file changed, 61 insertions(+), 19 deletions(-) diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index 1d0a46d..1c5d199 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -57,6 +57,16 @@ static int write_end_record(struct xc_sr_context *ctx) } /* + * Writes an CHECKPOINT record into the stream. + */ +static int write_checkpoint_record(struct xc_sr_context *ctx) +{ +struct xc_sr_record checkpoint = { REC_TYPE_CHECKPOINT, 0, NULL }; + +return write_record(ctx, &checkpoint); +} + +/* * Writes a batch of memory as a PAGE_DATA record into the stream. The batch * is constructed in ctx->save.batch_pfns. * @@ -467,6 +477,14 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap, &ctx->save.dirty_bitmap_hbuf); +/* + * With Remus, we will enter checkpointed save after live migration. + * In checkpointed save loop, we skip the live part and pause straight + * away to send dirty pages between checkpoints. + */ +if ( !ctx->save.live ) +goto last_iter; + rc = enable_logdirty(ctx); if ( rc ) goto out; @@ -505,6 +523,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) goto out; } + last_iter: rc = suspend_domain(ctx); if ( rc ) goto out; @@ -667,29 +686,52 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type) if ( rc ) goto err; -rc = ctx->save.ops.start_of_checkpoint(ctx); -if ( rc ) -goto err; +do { +rc = ctx->save.ops.start_of_checkpoint(ctx); +if ( rc ) +goto err; -if ( ctx->save.live ) -rc = send_domain_memory_live(ctx); -else -rc = send_domain_memory_nonlive(ctx); +if ( ctx->save.live || ctx->save.checkpointed ) +rc = send_domain_memory_live(ctx); +else +rc = send_domain_memory_nonlive(ctx); -if ( rc ) -goto err; +if ( rc ) +goto err; -if ( !ctx->dominfo.shutdown || - (ctx->dominfo.shutdown_reason != SHUTDOWN_suspend) ) -{ -ERROR("Domain has not been suspended"); -rc = -1; -goto err; -} +if ( !ctx->dominfo.shutdown || +(ctx->dominfo.shutdown_reason != SHUTDOWN_suspend) ) +{ +ERROR("Domain has not been suspended"); +rc = -1; +goto err; +} -rc = ctx->save.ops.end_of_checkpoint(ctx); -if ( rc ) -goto err; +rc = ctx->save.ops.end_of_checkpoint(ctx); +if ( rc ) +goto err; + +if ( ctx->save.checkpointed ) +{ +if ( ctx->save.live ) +{ +/* End of live migration, we are sending checkpointed stream */ +ctx->save.live = false; +} + +rc = write_checkpoint_record(ctx); +if ( rc ) +goto err; + +ctx->save.callbacks->postcopy(ctx->save.callbacks->data); + +rc = ctx->save.callbacks->checkpoint(ctx->save.callbacks->data); +if ( rc > 0 ) +xc_report_progress_single(xch, "Checkpointed save"); +else +ctx->save.checkpointed = false; +} +} while ( ctx->save.checkpointed ); xc_report_progress_single(xch, "End of stream"); -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] pvgrub: initialise p2m_size
On Thu, 2015-05-14 at 03:56 +0100, Wei Liu wrote: > On Thu, May 14, 2015 at 01:51:18AM +0200, Samuel Thibault wrote: > > Wei Liu, le Sun 10 May 2015 14:14:51 +0100, a écrit : > > > In 84083790 ("libxc: add p2m_size to xc_dom_image") a new field is > > > added. We should initialised this field in pvgrub as well, otherwise > > > xc_dom_build_image won't work properly. > > > > > > Signed-off-by: Wei Liu > > > Cc: Ian Campbell > > > Cc: Ian Jackson > > > > Acked-by: Samuel Thibault > > > > Thanks for the ack! Ian already applied this one. > > Sorry I forgot to CC you when I sent this patch. I will remember doing > so next time. Likewise, I should have waited too. > > Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V2] xen/vm_event: Clean up control-register-write vm_events
At 11:40 +0300 on 14 May (1431603649), Razvan Cojocaru wrote: > As suggested by Andrew Cooper, this patch attempts to remove > some redundancy and allow for an easier time when adding vm_events > for new control registers in the future, by having a single > VM_EVENT_REASON_WRITE_CTRLREG vm_event type, meant to serve CR0, > CR3, CR4 and (newly introduced) XCR0. The actual control register > will be deduced by the new .index field in vm_event_write_ctrlreg > (renamed from vm_event_mov_to_cr). The patch has also modified the > xen-access.c test - it is now able to log CR3 events. > > Signed-off-by: Razvan Cojocaru Reviewed-by: Tim Deegan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC 2/7] linux-stubdomain: Compile Linux
On Fri, Feb 6, 2015 at 5:45 PM, Stefano Stabellini wrote: >> >> +LINUX_URL=ftp://ftp.kernel.org/pub/linux/kernel/v3.x/$(LINUX_V).tar.xz >> >> + >> >> +all: $(VMLINUZ) >> > >> > I think it is best if we git clone it. >> >> Is that still true if unpatched 3.18.6 works? I don't know if there >> is a desire to reduce load on kernel.org, for example. > > That's a good point. I think git clone would be more inline with any > other external project that we use. However I'll let the other > maintainers decide on this. It takes a *lng* time to download a full Linux git tree, and it takes up a huge amount of disk space. It would be a lot more convenient to be able to just download a tarball. -George ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 11/16] libxc/save: rename send_some_pages to send_dirty_pages
On 14/05/15 09:55, Yang Hongyang wrote: > rename send_some_pages to send_dirty_pages, no functional change. > > Signed-off-by: Yang Hongyang > CC: Ian Campbell > CC: Ian Jackson > CC: Wei Liu > CC: Andrew Cooper Reviewed-by: Andrew Cooper > --- > tools/libxc/xc_sr_save.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c > index adb5cce..3768bda 100644 > --- a/tools/libxc/xc_sr_save.c > +++ b/tools/libxc/xc_sr_save.c > @@ -379,8 +379,8 @@ static int send_all_pages(struct xc_sr_context *ctx) > * > * Bitmap is bounded by p2m_size. > */ > -static int send_some_pages(struct xc_sr_context *ctx, > - unsigned long entries) > +static int send_dirty_pages(struct xc_sr_context *ctx, > +unsigned long entries) > { > xc_interface *xch = ctx->xch; > xen_pfn_t p; > @@ -516,7 +516,7 @@ static int send_domain_memory_live(struct xc_sr_context > *ctx) > if ( rc ) > goto out; > > -rc = send_some_pages(ctx, stats.dirty_count); > +rc = send_dirty_pages(ctx, stats.dirty_count); > if ( rc ) > goto out; > } > @@ -541,7 +541,7 @@ static int send_domain_memory_live(struct xc_sr_context > *ctx) > > bitmap_or(dirty_bitmap, ctx->save.deferred_pages, ctx->save.p2m_size); > > -rc = send_some_pages(ctx, stats.dirty_count + > ctx->save.nr_deferred_pages); > +rc = send_dirty_pages(ctx, stats.dirty_count + > ctx->save.nr_deferred_pages); > if ( rc ) > goto out; > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 15/16] libxc/restore: introduce setup() and cleanup() on restore
On 14/05/15 09:55, Yang Hongyang wrote: > introduce setup() and cleanup() which subsume the > ctx->restore.ops.{setup,cleanup}() calls and also > do memory alloc/free. > > Signed-off-by: Yang Hongyang > CC: Andrew Cooper > CC: Ian Campbell > CC: Ian Jackson > CC: Wei Liu Reviewed-by: Andrew Cooper > --- > tools/libxc/xc_sr_restore.c | 48 > - > 1 file changed, 34 insertions(+), 14 deletions(-) > > diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c > index 8022c3d..2345f66 100644 > --- a/tools/libxc/xc_sr_restore.c > +++ b/tools/libxc/xc_sr_restore.c > @@ -510,6 +510,38 @@ static int process_record(struct xc_sr_context *ctx, > struct xc_sr_record *rec) > return rc; > } > > +static int setup(struct xc_sr_context *ctx) > +{ > +xc_interface *xch = ctx->xch; > +int rc; > + > +rc = ctx->restore.ops.setup(ctx); > +if ( rc ) > +goto err; > + > +ctx->restore.max_populated_pfn = (32 * 1024 / 4) - 1; > +ctx->restore.populated_pfns = bitmap_alloc( > +ctx->restore.max_populated_pfn + 1); > +if ( !ctx->restore.populated_pfns ) > +{ > +ERROR("Unable to allocate memory for populated_pfns bitmap"); > +rc = -1; > +goto err; > +} > + > + err: > +return rc; > +} > + > +static void cleanup(struct xc_sr_context *ctx) > +{ > +xc_interface *xch = ctx->xch; > + > +free(ctx->restore.populated_pfns); > +if ( ctx->restore.ops.cleanup(ctx) ) > +PERROR("Failed to clean up"); > +} > + > #ifdef XG_LIBXL_HVM_COMPAT > extern int read_qemu(struct xc_sr_context *ctx); > #endif > @@ -524,19 +556,10 @@ static int restore(struct xc_sr_context *ctx) > > IPRINTF("Restoring domain"); > > -rc = ctx->restore.ops.setup(ctx); > +rc = setup(ctx); > if ( rc ) > goto err; > > -ctx->restore.max_populated_pfn = (32 * 1024 / 4) - 1; > -ctx->restore.populated_pfns = bitmap_alloc( > -ctx->restore.max_populated_pfn + 1); > -if ( !ctx->restore.populated_pfns ) > -{ > -ERROR("Unable to allocate memory for populated_pfns bitmap"); > -goto err; > -} > - > do > { > rc = read_record(ctx, &rec); > @@ -571,10 +594,7 @@ static int restore(struct xc_sr_context *ctx) > PERROR("Restore failed"); > > done: > -free(ctx->restore.populated_pfns); > -rc = ctx->restore.ops.cleanup(ctx); > -if ( rc ) > -PERROR("Failed to clean up"); > +cleanup(ctx); > > if ( saved_rc ) > { ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v7 07/14] x86: dynamically get/set CBM for a domain
On Fri, 2015-05-08 at 16:56 +0800, Chao Peng wrote: > For CAT, COS is maintained in hypervisor only while CBM is exposed to > user space directly to allow getting/setting domain's cache capacity. > For each specified CBM, hypervisor will either use a existed COS which > has the same CBM or allocate a new one if the same CBM is not found. If > the allocation fails because of no enough COS available then error is > returned. The getting/setting are always operated on a specified socket. > For multiple sockets system, the interface may be called several times. > > Signed-off-by: Chao Peng > Reviewed-by: Dario Faggioli Just, one minor thing, only if you end up resending... > diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c > index 1feb2f6..385807b 100644 > --- a/xen/arch/x86/psr.c > +++ b/xen/arch/x86/psr.c > @@ -49,6 +49,14 @@ static unsigned int opt_cos_max = 255; > static uint64_t rmid_mask; > static DEFINE_PER_CPU(struct psr_assoc, psr_assoc); > > +static unsigned int get_socket_cpu(unsigned int socket) > +{ > +if ( socket < nr_sockets ) > +return cpumask_any(socket_to_cpumask[socket]); > + ... What about if ( likely(socket < nr_sockets) ) Regards, Dario signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH Remus v4 2/2] libxc/restore: implement Remus checkpointed restore
On 14/05/15 09:56, Yang Hongyang wrote: > With Remus, the restore flow should be: > the first full migration stream -> { periodically restore stream } > > Signed-off-by: Yang Hongyang > Signed-off-by: Andrew Cooper > CC: Ian Campbell > CC: Ian Jackson > CC: Wei Liu > --- > tools/libxc/xc_sr_common.h | 14 + > tools/libxc/xc_sr_restore.c | 121 > > 2 files changed, 125 insertions(+), 10 deletions(-) > > diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h > index f8121e7..3bf27f1 100644 > --- a/tools/libxc/xc_sr_common.h > +++ b/tools/libxc/xc_sr_common.h > @@ -208,6 +208,20 @@ struct xc_sr_context > /* Plain VM, or checkpoints over time. */ > bool checkpointed; > > +/* Currently buffering records between a checkpoint */ > +bool buffer_all_records; > + > +/* > + * With Remus, we buffer the records sent by the primary at checkpoint, > + * in case the primary will fail, we can recover from the last > + * checkpoint state. > + * This should be enough because primary only send dirty pages at > + * checkpoint. > + */ > +#define MAX_BUF_RECORDS 1024 > +struct xc_sr_record *buffered_records; > +unsigned buffered_rec_num; > + > /* > * Xenstore and Console parameters. > * INPUT: evtchn & domid > diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c > index 9ab5760..3c93406 100644 > --- a/tools/libxc/xc_sr_restore.c > +++ b/tools/libxc/xc_sr_restore.c > @@ -468,10 +468,73 @@ static int handle_page_data(struct xc_sr_context *ctx, > struct xc_sr_record *rec) > return rc; > } > > +static int process_record(struct xc_sr_context *ctx, struct xc_sr_record > *rec); > +static int handle_checkpoint(struct xc_sr_context *ctx) > +{ > +xc_interface *xch = ctx->xch; > +int rc = 0; > +unsigned i; > +struct xc_sr_record *rec; > + > +if ( !ctx->restore.checkpointed ) > +{ > +ERROR("Found checkpoint in non-checkpointed stream"); > +rc = -1; > +goto err; > +} > + > +if ( ctx->restore.buffer_all_records ) > +{ > +IPRINTF("All records buffered"); > + > +/* > + * We need to set buffer_all_records to false in > + * order to process records instead of buffer records. > + * buffer_all_records should be set back to true after > + * we successfully processed all records. > + */ > +ctx->restore.buffer_all_records = false; > +for ( i = 0; i < ctx->restore.buffered_rec_num; i++) Space before closing bracket. > +{ > +rec = ctx->restore.buffered_records + > +i * sizeof(struct xc_sr_record); This pointer arithmetic looks wrong. FWIW, "rec = &ctx->restore.buffered_records[i];" would be clearer, although you don't even need to pull it into a variable as it is only referenced once. > +rc = process_record(ctx, rec); > +if ( rc ) > +goto err; > +} > +ctx->restore.buffered_rec_num = 0; > +ctx->restore.buffer_all_records = true; > +IPRINTF("All records processed"); > +} > +else > +ctx->restore.buffer_all_records = true; > + > + err: > +return rc; > +} > + > static int process_record(struct xc_sr_context *ctx, struct xc_sr_record > *rec) > { > xc_interface *xch = ctx->xch; > int rc = 0; > +struct xc_sr_record *buf_rec; > + > +if ( ctx->restore.buffer_all_records && > + rec->type != REC_TYPE_END && > + rec->type != REC_TYPE_CHECKPOINT ) > +{ > +if ( ctx->restore.buffered_rec_num >= MAX_BUF_RECORDS ) > +{ > +ERROR("There are too many records within a checkpoint"); > +return -1; > +} > + > +buf_rec = ctx->restore.buffered_records + > + ctx->restore.buffered_rec_num++ * sizeof(struct > xc_sr_record); Ah - this is how the other bit of pointer arithmetic doesn’t break, but it will wander off the array if the sender provides more than 32 records. > +memcpy(buf_rec, rec, sizeof(struct xc_sr_record)); As before, memcpy(&ctx->restore.buffered_records[ctx->restore.buffered_rec_num++], rec, sizeof(*rec)); might be a little more simple. > + > +return 0; > +} > > switch ( rec->type ) > { > @@ -487,12 +550,17 @@ static int process_record(struct xc_sr_context *ctx, > struct xc_sr_record *rec) > ctx->restore.verify = true; > break; > > +case REC_TYPE_CHECKPOINT: > +rc = handle_checkpoint(ctx); > +break; > + > default: > rc = ctx->restore.ops.process_record(ctx, rec); > break; > } > > free(rec->data); > +rec->data = NULL; > > if ( rc == RECORD_NOT_PROCESSED ) > { > @@ -529,6 +597,15 @@ static int setup(struct xc_sr_context *ctx) >
Re: [Xen-devel] [RFC 2/7] linux-stubdomain: Compile Linux
On Thu, 2015-05-14 at 10:08 +0100, George Dunlap wrote: > On Fri, Feb 6, 2015 at 5:45 PM, Stefano Stabellini > wrote: > >> >> +LINUX_URL=ftp://ftp.kernel.org/pub/linux/kernel/v3.x/$(LINUX_V).tar.xz > >> >> + > >> >> +all: $(VMLINUZ) > >> > > >> > I think it is best if we git clone it. > >> > >> Is that still true if unpatched 3.18.6 works? I don't know if there > >> is a desire to reduce load on kernel.org, for example. > > > > That's a good point. I think git clone would be more inline with any > > other external project that we use. However I'll let the other > > maintainers decide on this. > > It takes a *lng* time to download a full Linux git tree, and > it takes up a huge amount of disk space. It would be a lot more > convenient to be able to just download a tarball. git clone --depth= to create a shallow clone? Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 1/4] xen: introduce a helper to allocate non-contiguous memory
At 16:25 +0200 on 11 May (1431361525), Roger Pau Monne wrote: > The allocator uses independent calls to alloc_domheap_pages in order to get > the desired amount of memory and then maps all the independent physical > addresses into a contiguous virtual address space. > > Signed-off-by: Roger Pau Monné > Tested-by: Julien Grall (ARM) [...] > +void *vzalloc(size_t size) > +{ > +void *p = vmalloc(size); > + > +return p ? memset(p, 0, size) : p; > +} Should this be using clear_page()? Our plain memset() is not optimized for SSE &c. Apart from that, this patch looks very nice indeed. Cheers, Tim. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 2/4] xen/shadow: fix shadow_track_dirty_vram to work on hvm guests
At 16:25 +0200 on 11 May (1431361526), Roger Pau Monne wrote: > Modify shadow_track_dirty_vram to use a local buffer and then flush to the > guest without the paging_lock held. This is modeled after > hap_track_dirty_vram. > > Signed-off-by: Roger Pau Monné Reviewed-by: Tim Deegan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 4/4] xen: rework paging_log_dirty_op to work with hvm guests
At 16:25 +0200 on 11 May (1431361528), Roger Pau Monne wrote: > When the caller of paging_log_dirty_op is a hvm guest Xen would choke when > trying to copy the dirty bitmap to the guest because the paging lock is > already held. > > Fix this by independently mapping each page of the guest bitmap as needed > without the paging lock held. > > Signed-off-by: Roger Pau Monné This looks mostly correct, but OTOH we now seem to have two nested stop-and-retry mechsnisms in one function. I think this would be cleaner if here: > +if ( pages >> (3 + PAGE_SHIFT) != > + index_mapped >> (3 + PAGE_SHIFT) ) > { > -rv = -EFAULT; > -goto out; > +/* We need to map next page */ > +d->arch.paging.preempt.log_dirty.i4 = i4; > +d->arch.paging.preempt.log_dirty.i3 = i3; > +d->arch.paging.preempt.log_dirty.i2 = i2; > +d->arch.paging.preempt.log_dirty.done = pages; > +paging_unlock(d); > +unmap_dirty_bitmap(dirty_bitmap, page); we set the rest of the preempt state, like so: d->arch.paging.preempt.dom = current->domain; d->arch.paging.preempt.op = sc->op; resuming = 1; and bailed to the very top of the function. I think we might also need a hypercall_preempt_check() here as well, so that this restart path doesn't stop us ever reaching the preempt_checks below. Cheers, Tim. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH 5/6] ts-xen-build-prep: mkfs a new /home/osstest, don't resize2fs
On Wed, 2015-05-13 at 12:15 +0100, Ian Jackson wrote: > +sub replace_home () { > +my $dir = '/home/osstest'; > +my $mapper = lv_dev_mapper($vg,$lvleaf); > +my ($fstype,@opts) = qw(ext3 -m 0 -O sparse_super); > +target_cmd_root($ho, < +set -ex > + if mount | sed -e 's/^[^ ].* on //; s/ .*//' | grep -F '$dir'; then > + exit 0 > + fi > + mkfs -t $fstype @opts $lv > +mount $lv /mnt > + rsync -aHx --numeric-ids $dir/. /mnt/. > + rm -rf $dir > + mkdir -m 2700 $dir > + echo '$mapper $dir $fstype defaults 0 0' >>/etc/fstab > + umount /mnt > + mount $dir This doesn't update /etc/fstab so in standalone mode (when a host may share build and test duties) the reboots will cause the old /home/osstest to reappear and then I'm not sure if this all does what we want or not with the existing lv. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH Remus v5 2/2] libxc/restore: implement Remus checkpointed restore
With Remus, the restore flow should be: the first full migration stream -> { periodically restore stream } Signed-off-by: Yang Hongyang Signed-off-by: Andrew Cooper CC: Ian Campbell CC: Ian Jackson CC: Wei Liu --- tools/libxc/xc_sr_common.h | 14 ++ tools/libxc/xc_sr_restore.c | 113 2 files changed, 117 insertions(+), 10 deletions(-) diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h index f8121e7..3bf27f1 100644 --- a/tools/libxc/xc_sr_common.h +++ b/tools/libxc/xc_sr_common.h @@ -208,6 +208,20 @@ struct xc_sr_context /* Plain VM, or checkpoints over time. */ bool checkpointed; +/* Currently buffering records between a checkpoint */ +bool buffer_all_records; + +/* + * With Remus, we buffer the records sent by the primary at checkpoint, + * in case the primary will fail, we can recover from the last + * checkpoint state. + * This should be enough because primary only send dirty pages at + * checkpoint. + */ +#define MAX_BUF_RECORDS 1024 +struct xc_sr_record *buffered_records; +unsigned buffered_rec_num; + /* * Xenstore and Console parameters. * INPUT: evtchn & domid diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c index 9ab5760..8468ffc 100644 --- a/tools/libxc/xc_sr_restore.c +++ b/tools/libxc/xc_sr_restore.c @@ -468,11 +468,69 @@ static int handle_page_data(struct xc_sr_context *ctx, struct xc_sr_record *rec) return rc; } +static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec); +static int handle_checkpoint(struct xc_sr_context *ctx) +{ +xc_interface *xch = ctx->xch; +int rc = 0; +unsigned i; + +if ( !ctx->restore.checkpointed ) +{ +ERROR("Found checkpoint in non-checkpointed stream"); +rc = -1; +goto err; +} + +if ( ctx->restore.buffer_all_records ) +{ +IPRINTF("All records buffered"); + +/* + * We need to set buffer_all_records to false in + * order to process records instead of buffer records. + * buffer_all_records should be set back to true after + * we successfully processed all records. + */ +ctx->restore.buffer_all_records = false; +for ( i = 0; i < ctx->restore.buffered_rec_num; i++ ) +{ +rc = process_record(ctx, &ctx->restore.buffered_records[i]); +if ( rc ) +goto err; +} +ctx->restore.buffered_rec_num = 0; +ctx->restore.buffer_all_records = true; +IPRINTF("All records processed"); +} +else +ctx->restore.buffer_all_records = true; + + err: +return rc; +} + static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec) { xc_interface *xch = ctx->xch; int rc = 0; +if ( ctx->restore.buffer_all_records && + rec->type != REC_TYPE_END && + rec->type != REC_TYPE_CHECKPOINT ) +{ +if ( ctx->restore.buffered_rec_num >= MAX_BUF_RECORDS ) +{ +ERROR("There are too many records within a checkpoint"); +return -1; +} + +memcpy(&ctx->restore.buffered_records[ctx->restore.buffered_rec_num++], + rec, sizeof(*rec)); + +return 0; +} + switch ( rec->type ) { case REC_TYPE_END: @@ -487,12 +545,17 @@ static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec) ctx->restore.verify = true; break; +case REC_TYPE_CHECKPOINT: +rc = handle_checkpoint(ctx); +break; + default: rc = ctx->restore.ops.process_record(ctx, rec); break; } free(rec->data); +rec->data = NULL; if ( rc == RECORD_NOT_PROCESSED ) { @@ -529,6 +592,15 @@ static int setup(struct xc_sr_context *ctx) goto err; } +ctx->restore.buffered_records = malloc( +MAX_BUF_RECORDS * sizeof(struct xc_sr_record)); +if ( !ctx->restore.buffered_records ) +{ +ERROR("Unable to allocate memory for buffered records"); +rc = -1; +goto err; +} + err: return rc; } @@ -536,7 +608,12 @@ static int setup(struct xc_sr_context *ctx) static void cleanup(struct xc_sr_context *ctx) { xc_interface *xch = ctx->xch; +unsigned i; + +for ( i = 0; i < ctx->restore.buffered_rec_num; i++ ) +free(ctx->restore.buffered_records[i].data); +free(ctx->restore.buffered_records); free(ctx->restore.populated_pfns); if ( ctx->restore.ops.cleanup(ctx) ) PERROR("Failed to clean up"); @@ -564,7 +641,27 @@ static int restore(struct xc_sr_context *ctx) { rc = read_record(ctx, &rec); if ( rc ) -goto err; +{ +if ( ctx->restore.buffer_all_records ) +goto remus_failover; +else +
[Xen-devel] [PATCH Remus v5 1/2] libxc/save: implement Remus checkpointed save
With Remus, the save flow should be: live migration->{ periodically save(checkpointed save) } Signed-off-by: Yang Hongyang Reviewed-by: Andrew Cooper CC: Ian Campbell CC: Ian Jackson CC: Wei Liu --- tools/libxc/xc_sr_save.c | 80 1 file changed, 61 insertions(+), 19 deletions(-) diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index 1d0a46d..1c5d199 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -57,6 +57,16 @@ static int write_end_record(struct xc_sr_context *ctx) } /* + * Writes an CHECKPOINT record into the stream. + */ +static int write_checkpoint_record(struct xc_sr_context *ctx) +{ +struct xc_sr_record checkpoint = { REC_TYPE_CHECKPOINT, 0, NULL }; + +return write_record(ctx, &checkpoint); +} + +/* * Writes a batch of memory as a PAGE_DATA record into the stream. The batch * is constructed in ctx->save.batch_pfns. * @@ -467,6 +477,14 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap, &ctx->save.dirty_bitmap_hbuf); +/* + * With Remus, we will enter checkpointed save after live migration. + * In checkpointed save loop, we skip the live part and pause straight + * away to send dirty pages between checkpoints. + */ +if ( !ctx->save.live ) +goto last_iter; + rc = enable_logdirty(ctx); if ( rc ) goto out; @@ -505,6 +523,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) goto out; } + last_iter: rc = suspend_domain(ctx); if ( rc ) goto out; @@ -667,29 +686,52 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type) if ( rc ) goto err; -rc = ctx->save.ops.start_of_checkpoint(ctx); -if ( rc ) -goto err; +do { +rc = ctx->save.ops.start_of_checkpoint(ctx); +if ( rc ) +goto err; -if ( ctx->save.live ) -rc = send_domain_memory_live(ctx); -else -rc = send_domain_memory_nonlive(ctx); +if ( ctx->save.live || ctx->save.checkpointed ) +rc = send_domain_memory_live(ctx); +else +rc = send_domain_memory_nonlive(ctx); -if ( rc ) -goto err; +if ( rc ) +goto err; -if ( !ctx->dominfo.shutdown || - (ctx->dominfo.shutdown_reason != SHUTDOWN_suspend) ) -{ -ERROR("Domain has not been suspended"); -rc = -1; -goto err; -} +if ( !ctx->dominfo.shutdown || +(ctx->dominfo.shutdown_reason != SHUTDOWN_suspend) ) +{ +ERROR("Domain has not been suspended"); +rc = -1; +goto err; +} -rc = ctx->save.ops.end_of_checkpoint(ctx); -if ( rc ) -goto err; +rc = ctx->save.ops.end_of_checkpoint(ctx); +if ( rc ) +goto err; + +if ( ctx->save.checkpointed ) +{ +if ( ctx->save.live ) +{ +/* End of live migration, we are sending checkpointed stream */ +ctx->save.live = false; +} + +rc = write_checkpoint_record(ctx); +if ( rc ) +goto err; + +ctx->save.callbacks->postcopy(ctx->save.callbacks->data); + +rc = ctx->save.callbacks->checkpoint(ctx->save.callbacks->data); +if ( rc > 0 ) +xc_report_progress_single(xch, "Checkpointed save"); +else +ctx->save.checkpointed = false; +} +} while ( ctx->save.checkpointed ); xc_report_progress_single(xch, "End of stream"); -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [qemu-upstream-4.4-testing test] 55879: regressions - FAIL
flight 55879 qemu-upstream-4.4-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/55879/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail REGR. vs. 50323 Regressions which are regarded as allowable (not blocking): test-amd64-i386-libvirt 11 guest-start fail REGR. vs. 50323 test-amd64-i386-freebsd10-i386 16 guest-localmigrate/x10 fail like 50323 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 16 guest-stop fail never pass test-amd64-amd64-xl-qemuu-winxpsp3 16 guest-stop fail never pass version targeted for testing: qemuu50590fb0c104ac3ed7dc1817994ae1e11dcc6a3c baseline version: qemuu32c5e5e70be80417ca21aa6e632a81f899ceead1 People who touched revisions under test: Petr Matousek jobs: build-amd64-xend pass build-i386-xend pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass test-amd64-i386-xl pass test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-i386-freebsd10-amd64 pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-amd64-xl-credit2 pass test-amd64-i386-freebsd10-i386 fail test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-amd64-libvirt pass test-amd64-i386-libvirt fail test-amd64-amd64-xl-multivcpupass test-amd64-amd64-pairpass test-amd64-i386-pair pass test-amd64-amd64-xl-sedf-pin pass test-amd64-amd64-pv pass test-amd64-i386-pv pass test-amd64-amd64-xl-sedf pass test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 fail test-amd64-amd64-xl-qemuu-winxpsp3 fail sg-report-flight on osstest.test-lab.xenproject.org logs: /home/osstest/pub/logs images: /home/osstest/pub/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. commit 50590fb0c104ac3ed7dc1817994ae1e11dcc6a3c Author: Petr Matousek Date: Wed May 6 09:48:59 2015 +0200 fdc: force the fifo access to be in bounds of the allocated buffer During processing of certain commands such as FD_CMD_READ_ID and FD_CMD_DRIVE_SPECIFICATION_COMMAND the fifo memory access could get out of bounds leading to memory corruption with values coming from the guest. Fix this by making sure that the index is always bounded by the allocated memory. This is CVE-2015-3456. Signed-off-by: Petr Matousek Reviewed-by: John Snow ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH Remus v5 0/2] Remus support for Migration-v2
This patchset implement the Remus support for Migration v2 but without memory compressing. Git tree available at: https://github.com/macrosheep/xen/tree/Remus-newmig-v5 This patchset is based on [PATCH v6 00/16] Misc patches to aid migration v2 Remus support https://github.com/macrosheep/xen/tree/misc-remus-v6 Yang Hongyang (2): libxc/save: implement Remus checkpointed save libxc/restore: implement Remus checkpointed restore tools/libxc/xc_sr_common.h | 14 ++ tools/libxc/xc_sr_restore.c | 113 tools/libxc/xc_sr_save.c| 80 +++ 3 files changed, 178 insertions(+), 29 deletions(-) -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH Remus v5 2/2] libxc/restore: implement Remus checkpointed restore
On 14/05/15 11:06, Yang Hongyang wrote: > With Remus, the restore flow should be: > the first full migration stream -> { periodically restore stream } > > Signed-off-by: Yang Hongyang > Signed-off-by: Andrew Cooper > CC: Ian Campbell > CC: Ian Jackson > CC: Wei Liu Reviewed-by: Andrew Cooper > --- > tools/libxc/xc_sr_common.h | 14 ++ > tools/libxc/xc_sr_restore.c | 113 > > 2 files changed, 117 insertions(+), 10 deletions(-) > > diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h > index f8121e7..3bf27f1 100644 > --- a/tools/libxc/xc_sr_common.h > +++ b/tools/libxc/xc_sr_common.h > @@ -208,6 +208,20 @@ struct xc_sr_context > /* Plain VM, or checkpoints over time. */ > bool checkpointed; > > +/* Currently buffering records between a checkpoint */ > +bool buffer_all_records; > + > +/* > + * With Remus, we buffer the records sent by the primary at checkpoint, > + * in case the primary will fail, we can recover from the last > + * checkpoint state. > + * This should be enough because primary only send dirty pages at > + * checkpoint. > + */ > +#define MAX_BUF_RECORDS 1024 > +struct xc_sr_record *buffered_records; > +unsigned buffered_rec_num; > + > /* > * Xenstore and Console parameters. > * INPUT: evtchn & domid > diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c > index 9ab5760..8468ffc 100644 > --- a/tools/libxc/xc_sr_restore.c > +++ b/tools/libxc/xc_sr_restore.c > @@ -468,11 +468,69 @@ static int handle_page_data(struct xc_sr_context *ctx, > struct xc_sr_record *rec) > return rc; > } > > +static int process_record(struct xc_sr_context *ctx, struct xc_sr_record > *rec); > +static int handle_checkpoint(struct xc_sr_context *ctx) > +{ > +xc_interface *xch = ctx->xch; > +int rc = 0; > +unsigned i; > + > +if ( !ctx->restore.checkpointed ) > +{ > +ERROR("Found checkpoint in non-checkpointed stream"); > +rc = -1; > +goto err; > +} > + > +if ( ctx->restore.buffer_all_records ) > +{ > +IPRINTF("All records buffered"); > + > +/* > + * We need to set buffer_all_records to false in > + * order to process records instead of buffer records. > + * buffer_all_records should be set back to true after > + * we successfully processed all records. > + */ > +ctx->restore.buffer_all_records = false; > +for ( i = 0; i < ctx->restore.buffered_rec_num; i++ ) > +{ > +rc = process_record(ctx, &ctx->restore.buffered_records[i]); > +if ( rc ) > +goto err; > +} > +ctx->restore.buffered_rec_num = 0; > +ctx->restore.buffer_all_records = true; > +IPRINTF("All records processed"); > +} > +else > +ctx->restore.buffer_all_records = true; > + > + err: > +return rc; > +} > + > static int process_record(struct xc_sr_context *ctx, struct xc_sr_record > *rec) > { > xc_interface *xch = ctx->xch; > int rc = 0; > > +if ( ctx->restore.buffer_all_records && > + rec->type != REC_TYPE_END && > + rec->type != REC_TYPE_CHECKPOINT ) > +{ > +if ( ctx->restore.buffered_rec_num >= MAX_BUF_RECORDS ) > +{ > +ERROR("There are too many records within a checkpoint"); > +return -1; > +} > + > + > memcpy(&ctx->restore.buffered_records[ctx->restore.buffered_rec_num++], > + rec, sizeof(*rec)); > + > +return 0; > +} > + > switch ( rec->type ) > { > case REC_TYPE_END: > @@ -487,12 +545,17 @@ static int process_record(struct xc_sr_context *ctx, > struct xc_sr_record *rec) > ctx->restore.verify = true; > break; > > +case REC_TYPE_CHECKPOINT: > +rc = handle_checkpoint(ctx); > +break; > + > default: > rc = ctx->restore.ops.process_record(ctx, rec); > break; > } > > free(rec->data); > +rec->data = NULL; > > if ( rc == RECORD_NOT_PROCESSED ) > { > @@ -529,6 +592,15 @@ static int setup(struct xc_sr_context *ctx) > goto err; > } > > +ctx->restore.buffered_records = malloc( > +MAX_BUF_RECORDS * sizeof(struct xc_sr_record)); > +if ( !ctx->restore.buffered_records ) > +{ > +ERROR("Unable to allocate memory for buffered records"); > +rc = -1; > +goto err; > +} > + > err: > return rc; > } > @@ -536,7 +608,12 @@ static int setup(struct xc_sr_context *ctx) > static void cleanup(struct xc_sr_context *ctx) > { > xc_interface *xch = ctx->xch; > +unsigned i; > + > +for ( i = 0; i < ctx->restore.buffered_rec_num; i++ ) > +free(ctx->restore.buffered_records[i].data); > > +free(ctx->restore.buffered_records); > free(
Re: [Xen-devel] [RFC][PATCH 07/13] xen/passthrough: extend hypercall to support rdm reservation policy
Hi Chen, On 14/05/15 06:47, Chen, Tiejun wrote: >> This assign_device callback deals with the latter. So from the name the >> value doesn't look right. >> > > What about XEN_DOMCTL_DEV_RDM_XXX? That would be better naming. I would also add a XEN_DOMCTL_DEV_NO_RDM that would be use for non-PCI assignment Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] libxl: assigned a default ssid_label (XSM label) to guests
system_u:system_r:domU_t is defined in the default policy and makes as much sense as anything for a default. This change required moving the call to domain_create_info_setdefault to be before the ssid_label is translated into ssidref, which also moves it before some other stuff which consumes things from c_info, which is correct since setdefault should always be called first. Apart from the SSID handling there should be no functional change (since setdefault doesn't actually act on anything which that other stuff uses). There is no need to set exec_ssid_label since the default is to leave the domain using the ssid_label after build. I haven't done anything with the device model ssid. Signed-off-by: Ian Campbell Cc: Daniel De Graaf Cc: wei.l...@citrix.com --- docs/man/xl.cfg.pod.5 |4 +++- tools/libxl/libxl_create.c | 11 --- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 index 8e4154f..fcca1cc 100644 --- a/docs/man/xl.cfg.pod.5 +++ b/docs/man/xl.cfg.pod.5 @@ -437,7 +437,9 @@ UUID will be generated. =item B -Assign an XSM security label to this domain. +Assign an XSM security label to this domain. By default a domain is +assigned the label B, which is defined in +the default policy. =item B diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index f0da7dc..4dd2ec2 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -42,6 +42,11 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc, libxl_defbool_setdefault(&c_info->run_hotplug_scripts, true); libxl_defbool_setdefault(&c_info->driver_domain, false); +if (!c_info->ssid_label) { +c_info->ssid_label = libxl__strdup(NOGC, "system_u:system_r:domU_t"); +LOG(INFO, "Using default ssid_label: %s", c_info->ssid_label); +} + return 0; } @@ -802,6 +807,9 @@ static void initiate_domain_create(libxl__egc *egc, domid = 0; +ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info); +if (ret) goto error_out; + if (d_config->c_info.ssid_label) { char *s = d_config->c_info.ssid_label; ret = libxl_flask_context_to_sid(ctx, s, strlen(s), @@ -887,9 +895,6 @@ static void initiate_domain_create(libxl__egc *egc, goto error_out; } -ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info); -if (ret) goto error_out; - ret = libxl__domain_make(gc, d_config, &domid, &state->config); if (ret) { LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot make domain: %d", ret); -- 1.7.10.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xenpm: Initialize cputopo pointer
On Thu, 2015-05-14 at 04:02 +0100, Wei Liu wrote: > On Wed, May 13, 2015 at 01:37:35PM -0400, Boris Ostrovsky wrote: > > Commit 250f0b43af1a ("libxl/libxc: Move libxl_get_cpu_topology()'s > > hypercall buffer management to libxc") broke non-debug compilation: > > on error path we may have uninitialized cputopo pointer. > > > > Signed-off-by: Boris Ostrovsky > > Reported-by: Olaf Hering > > Acked-by: Wei Liu Acked + applied, thanks. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv4 3/5] xen: use ticket locks for spin locks
At 15:37 +0100 on 11 May (1431358623), David Vrabel wrote: > Replace the byte locks with ticket locks. Ticket locks are: a) fair; > and b) peform better when contented since they spin without an atomic > operation. > > The lock is split into two ticket values: head and tail. A locker > acquires a ticket by (atomically) increasing tail and using the > previous tail value. A CPU holds the lock if its ticket == head. The > lock is released by increasing head. > > spin_lock_irq() and spin_lock_irqsave() now spin with irqs disabled > (previously, they would spin with irqs enabled if possible). This is > required to prevent deadlocks when the irq handler tries to take the > same lock with a higher ticket. > > Architectures need only provide arch_fetch_and_add() and two barriers: > arch_lock_acquire_barrier() and arch_lock_release_barrier(). > > Signed-off-by: David Vrabel Looking good. I have two nits about comments, and one bug, which I missed in the last version: > int _spin_trylock(spinlock_t *lock) > { > +spinlock_tickets_t old, new; > + > check_lock(&lock->debug); > -if ( !_raw_spin_trylock(&lock->raw) ) > +old = observe_lock(&lock->tickets); > +if ( old.head != old.tail ) > return 0; > +new = old; > +new.tail++; > +if ( cmpxchg(&lock->tickets.head_tail, > + old.head_tail, new.head_tail) != old.head_tail ) > +{ > +/* > + * cmpxchg() is a full barrier so no need for an > + * arch_lock_acquire_barrier(). > + */ This comment belongs in the other branch, where we have taken the lock. > +return 0; > +} > #ifdef LOCK_PROFILE > if (lock->profile) > lock->profile->time_locked = NOW(); > @@ -213,27 +219,32 @@ int _spin_trylock(spinlock_t *lock) > > void _spin_barrier(spinlock_t *lock) > { > +spinlock_tickets_t sample; > #ifdef LOCK_PROFILE > s_time_t block = NOW(); > -u64 loop = 0; > +#endif > > check_barrier(&lock->debug); > -do { smp_mb(); loop++;} while ( _raw_spin_is_locked(&lock->raw) ); > -if ((loop > 1) && lock->profile) > +smp_mb(); > +sample = observe_lock(&lock->tickets); > +if ( sample.head != sample.tail ) > { > -lock->profile->time_block += NOW() - block; > -lock->profile->block_cnt++; > -} > -#else > -check_barrier(&lock->debug); > -do { smp_mb(); } while ( _raw_spin_is_locked(&lock->raw) ); > +while ( observe_head(&lock->tickets) != sample.tail ) This test should be "observe_head(&lock->tickets) == sample.head", i.e. wait until the thread that held the lock has released it. Checking for it to reach the tail is unnecessary (other threads that were queueing for the lock at the sample time don't matter) and dangerous (on a contended lock head might pass sample.tail without us happening to observe it being == ). > +cpu_relax(); > +#ifdef LOCK_PROFILE > +if ( lock->profile ) > +{ > +lock->profile->time_block += NOW() - block; > +lock->profile->block_cnt++; > +} > #endif > +} > smp_mb(); > } [...] > --- a/xen/include/asm-x86/system.h > +++ b/xen/include/asm-x86/system.h > @@ -185,6 +185,9 @@ static always_inline unsigned long __xadd( > #define set_mb(var, value) do { xchg(&var, value); } while (0) > #define set_wmb(var, value) do { var = value; wmb(); } while (0) > > +#define arch_lock_acquire_barrier() barrier() > +#define arch_lock_release_barrier() barrier() This could do with a comment explaining why they're not smp_mb(). Summarizing the last thread: on x86 the only reordering is of reads with older writes. In the lock case, the read in observe_head() can only be reordered with writes that precede it, and moving a write _into_ a locked section is OK. In the release case, the write in add_sized() can only be reordered with reads that follow it, and hoisting a read _into_ a locked region is OK. Cheers, Tim. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] make-flight: Drop pointless jobs from seabios and ovmf flights
Ian Campbell writes ("[PATCH] make-flight: Drop pointless jobs from seabios and ovmf flights"): > These are both firmware which is used only with qemuu tests. There is > no point testing either PV guets or HVM with qemut (which always uses > rombios). > > Comparing before and after of: Acked-by: Ian Jackson ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH OSSTEST] sg-run-job: Repeat rumpuserxen-demo-xenstorels test 150 times
Ian Campbell writes ("[PATCH OSSTEST] sg-run-job: Repeat rumpuserxen-demo-xenstorels test 150 times"): > (Was: Re: [osstest test] 50423: regressions - FAIL) ... > I thought I'd sent such a patch, but no sign of it in the relevant > folders. Here's another. Acked-by: Ian Jackson > I'm unsure if the testid should change, I think not in which case this > will take a force push to clear the is-a-regression-ness. IMO it should not, and indeed. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt test] 55257: regressions - FAIL
On Wed, 2015-05-13 at 18:46 +0100, Anthony PERARD wrote: > On Wed, May 13, 2015 at 09:46:28AM +0100, Ian Campbell wrote: > > On Mon, 2015-05-11 at 10:36 -0600, Jim Fehlig wrote: > > [...] > > > > The qemu log is sadly empty so I've no clue why this timed out. > > > > > > > > > > I guess qemu didn't run at all... > > > > > > > Perhaps there is something in > > > > http://logs.test-lab.xenproject.org/osstest/logs/55257/test-amd64-amd64-libvirt/merlot1---var-log-libvirt-libvirtd.log.gz > > > > I can't make heads nor tail though. > > > > > > > > > > Nothing interesting. Only the unhelpful > > > > > > 2015-05-11 12:42:17.451+: 4280: error : libxlDomainStart:1032 : > > > internal error: libxenlight failed to create new domain > > > 'debian.guest.osstest' > > > > This happened again in > > http://logs.test-lab.xenproject.org/osstest/logs/55349/test-amd64-amd64-libvirt/info.html > > > > Is there anything we could tweak in osstest to produce more helpful > > logging? > > Well we can find in var-log-libvirt-libvirtd.log.gz this: > 2015-05-12 17:39:35.180+: 4329: error : libxlDomainStart:1032 : internal > error: libxenlight failed to create new domain 'debian.guest.osstest' > > And for more information we need to look into the driver specific log, > libxl logs in var-log-libvirt-libxl-libxl-driver.log: > libxl: error: libxl_exec.c:393:spawn_watch_event: domain 1 device model: > startup timed out Thanks, all of that was mentioned earlier in the thread too, I was looking for ways to get more info. > I'm seeing this error a lot on our OpenStack CI loop, I thought the error > was due to the "host" been very busy, but if osstest is having the same > issue, then there is probably something wrong with libxl+libvirt :(. Are you able to reproduce at will or is it like osstest and just a sporadic failure? I suppose the openstack CI loop doesn't capture anything more interesting than osstest does? FWIW http://logs.test-lab.xenproject.org/osstest/logs/55443/ seems to have two more instances of this (amd64 and i386), but with no interesting logs still and a different one on ARM: http://logs.test-lab.xenproject.org/osstest/logs/55443/test-armhf-armhf-libvirt/11.ts-guest-start.log: 2015-05-13 09:23:32.193+: 16389: info : libvirt version: 1.2.16 2015-05-13 09:23:32.193+: 16389: warning : virKeepAliveTimerInternal:143 : No response from client 0xb7000c38 after 6 keepalive messages in 35 seconds 2015-05-13 09:23:32.193+: 16390: warning : virKeepAliveTimerInternal:143 : No response from client 0xb7000c38 after 6 keepalive messages in 35 seconds error: Failed to create domain from /etc/xen/debian.guest.osstest.cfg.xml error: internal error: received hangup / error event on socket In that case the the libxl-driver log ends with: libxl: debug: libxl_dm.c:1495:libxl__spawn_local_dm: Spawning device-model /usr/local/lib/xen/bin/qemu-system-i386 with arguments: [...] libxl: debug: libxl_event.c:600:libxl__ev_xswatch_register: watch w=0xb2e07bcc wpath=/local/domain/0/device-model/1/state token=3/0: register slotnum=3 libxl: debug: libxl_create.c:1560:do_domain_create: ao 0xb2e044f0: inprogress: poller=0xb2e07590, flags=i libxl: debug: libxl_event.c:537:watchfd_callback: watch w=0xb2e07bcc wpath=/local/domain/0/device-model/1/state token=3/0: event epath=/local/domain/0/device-model/1/state Which I don't think is complete, i.e. there should be more? Not sure if this gives a hint for the x86 case too? I don't see anything useful in http://logs.test-lab.xenproject.org/osstest/logs/55443/test-armhf-armhf-libvirt/arndale-lakeside---var-log-libvirt-libvirtd.log.gz Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH][RFC] libxl: Add AHCI support for upstream qemu
Il 13/05/2015 14:54, Fabio Fantoni ha scritto: Usage: ahci=0|1 (default=0) If enabled adds ich9 disk controller in ahci mode and uses it with upstream qemu to emulate disks instead of ide. It doesn't support cdroms which still use ide (cdroms will use "-device ide-cd" as new qemu parameters) Ahci requires new qemu parameters but for now other emulated disks cases remains with old ones because automatic bus selection seems bugged in qemu using new parameters on ide controller. Probably I don't remember good, there is bus selection problem with multiple disks with ahci: qemu-system-i386: -device ide-hd,drive=ahcidisk-1: Can't create IDE unit 1, bus supports only 1 units qemu-system-i386: -device ide-hd,drive=ahcidisk-1: Device initialization failed. qemu-system-i386: -device ide-hd,drive=ahcidisk-1: Device 'ide-hd' could not be initialized Force to unit=0 all don't solves the problem: qemu-system-i386: -device ide-hd,unit=0,drive=ahcidisk-1: IDE unit 0 is in use qemu-system-i386: -device ide-hd,unit=0,drive=ahcidisk-1: Device initialization failed. qemu-system-i386: -device ide-hd,unit=0,drive=ahcidisk-1: Device 'ide-hd' could not be initialized Bus definition is needed to have it full working (already test, I'll post v2 with it) Is there any problem in xen with disk bus "not automatic"? I'll retry also conversion to new qemu parameters for other disk emulation cases and I'll post the patch if will works. NOTES: This patch is a only a fast draft for testing. Tested with ubuntu 15.04 hvm, windows 7 and windows 8 domUs. Doc entry and libxl.h define should be added, I'll do. Other emulated disks cases should be converted to use new qemu parameters but probably a fix in qemu is needed. Any comment is appreciated. Signed-off-by: Fabio Fantoni --- tools/libxl/libxl_create.c | 1 + tools/libxl/libxl_dm.c | 10 +- tools/libxl/libxl_types.idl | 1 + tools/libxl/xl_cmdimpl.c| 1 + 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index f0da7dc..fcfe24a 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -322,6 +322,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm, false); libxl_defbool_setdefault(&b_info->u.hvm.usb,false); libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true); +libxl_defbool_setdefault(&b_info->u.hvm.ahci, false); libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false); if (!libxl_defbool_val(b_info->u.hvm.spice.enable) && diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 0c6408d..1b48cc9 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -804,6 +804,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size)); if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) { +if (libxl_defbool_val(b_info->u.hvm.ahci)) +flexarray_append_pair(dm_args, "-device", "ahci"); for (i = 0; i < num_disks; i++) { int disk, part; int dev_number = @@ -858,7 +860,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, drive = libxl__sprintf (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback", pdev_path, disk, format); -else if (disk < 4) +else if (disk < 6 && libxl_defbool_val(b_info->u.hvm.ahci)){ +flexarray_vappend(dm_args, "-drive", + GCSPRINTF("file=%s,if=none,id=ahcidisk-%d,format=%s,cache=writeback", +pdev_path, disk, format), "-device", GCSPRINTF("ide-hd,drive=ahcidisk-%d", +disk), NULL); +continue; +}else if (disk < 4) drive = libxl__sprintf (gc, "file=%s,if=ide,index=%d,media=disk,format=%s,cache=writeback", pdev_path, disk, format); diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 023b21e..6326429 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -433,6 +433,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("nested_hvm", libxl_defbool), ("smbios_firmware", string), ("acpi_firmware",string), + ("ahci", libxl_defbool), ("nographic",libxl_defbool), ("vga", libxl_vga_interface_info), ("vnc", lib
Re: [Xen-devel] [PATCH v6 4/4] xen: rework paging_log_dirty_op to work with hvm guests
El 14/05/15 a les 11.53, Tim Deegan ha escrit: > At 16:25 +0200 on 11 May (1431361528), Roger Pau Monne wrote: >> When the caller of paging_log_dirty_op is a hvm guest Xen would choke when >> trying to copy the dirty bitmap to the guest because the paging lock is >> already held. >> >> Fix this by independently mapping each page of the guest bitmap as needed >> without the paging lock held. >> >> Signed-off-by: Roger Pau Monné > > This looks mostly correct, but OTOH we now seem to have two nested > stop-and-retry mechsnisms in one function. I think this would be > cleaner if here: > >> +if ( pages >> (3 + PAGE_SHIFT) != >> + index_mapped >> (3 + PAGE_SHIFT) ) >> { >> -rv = -EFAULT; >> -goto out; >> +/* We need to map next page */ >> +d->arch.paging.preempt.log_dirty.i4 = i4; >> +d->arch.paging.preempt.log_dirty.i3 = i3; >> +d->arch.paging.preempt.log_dirty.i2 = i2; >> +d->arch.paging.preempt.log_dirty.done = pages; >> +paging_unlock(d); >> +unmap_dirty_bitmap(dirty_bitmap, page); > > we set the rest of the preempt state, like so: > > d->arch.paging.preempt.dom = current->domain; > d->arch.paging.preempt.op = sc->op; > resuming = 1; > > and bailed to the very top of the function. I think we might also > need a hypercall_preempt_check() here as well, so that this restart > path doesn't stop us ever reaching the preempt_checks below. OK, I don't mind adding a preempt check here, although I think we would hit the ones below anyway as we go scanning the dirty bitmap even if we have to restart. Roger. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST Nested PATCH v10 3/9] Refactor installation of overlays
On Wed, 2015-05-13 at 11:36 +0800, longtao.pang wrote: > Based on Ian Campbell's v6_patch [04,05,06], I create this patch > to refactor installation of overlays for guest as well as host used. > > Link of Ian Campbell's patch: > http://lists.xenproject.org/archives/html/xen-devel/2015-05/msg00467.html > http://lists.xenproject.org/archives/html/xen-devel/2015-05/msg00452.html > http://lists.xenproject.org/archives/html/xen-devel/2015-05/msg00459.html FYI I've just pushed these to osstest's pretest branch where they will be tested and appear in production hopefully tomorrow if all goes well. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt test] 55257: regressions - FAIL
On Thu, May 14, 2015 at 11:47:18AM +0100, Ian Campbell wrote: > On Wed, 2015-05-13 at 18:46 +0100, Anthony PERARD wrote: > > On Wed, May 13, 2015 at 09:46:28AM +0100, Ian Campbell wrote: > > > On Mon, 2015-05-11 at 10:36 -0600, Jim Fehlig wrote: > > > [...] > > > > > The qemu log is sadly empty so I've no clue why this timed out. > > > > > > > > > > > > > I guess qemu didn't run at all... > > > > > > > > > Perhaps there is something in > > > > > http://logs.test-lab.xenproject.org/osstest/logs/55257/test-amd64-amd64-libvirt/merlot1---var-log-libvirt-libvirtd.log.gz > > > > > I can't make heads nor tail though. > > > > > > > > > > > > > Nothing interesting. Only the unhelpful > > > > > > > > 2015-05-11 12:42:17.451+: 4280: error : libxlDomainStart:1032 : > > > > internal error: libxenlight failed to create new domain > > > > 'debian.guest.osstest' > > > > > > This happened again in > > > http://logs.test-lab.xenproject.org/osstest/logs/55349/test-amd64-amd64-libvirt/info.html > > > > > > Is there anything we could tweak in osstest to produce more helpful > > > logging? > > > > Well we can find in var-log-libvirt-libvirtd.log.gz this: > > 2015-05-12 17:39:35.180+: 4329: error : libxlDomainStart:1032 : > > internal error: libxenlight failed to create new domain > > 'debian.guest.osstest' > > > > And for more information we need to look into the driver specific log, > > libxl logs in var-log-libvirt-libxl-libxl-driver.log: > > libxl: error: libxl_exec.c:393:spawn_watch_event: domain 1 device model: > > startup timed out > > Thanks, all of that was mentioned earlier in the thread too, I was > looking for ways to get more info. > > > I'm seeing this error a lot on our OpenStack CI loop, I thought the error > > was due to the "host" been very busy, but if osstest is having the same > > issue, then there is probably something wrong with libxl+libvirt :(. > > Are you able to reproduce at will or is it like osstest and just a > sporadic failure? Like osstest. I haven't spend much time on it yet, I did not try to reproduce it yet. > I suppose the openstack CI loop doesn't capture anything more > interesting than osstest does? No, nothing else interesting. The next step would be to enable more debug output from libvirtd by playing with "log_level" and "log_filters" in /etc/libvirtd.conf, but I don't know which filter would be intersting. -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
Usage: ahci=0|1 (default=0) If enabled adds ich9 disk controller in ahci mode and uses it with upstream qemu to emulate disks instead of ide. It doesn't support cdroms which still use ide (cdroms will use "-device ide-cd" as new qemu parameters) Ahci requires new qemu parameters but for now other emulated disks cases remains with old ones because automatic bus selection seems bugged in qemu using new parameters. (I'll retry) NOTES: This patch is a only a fast draft for testing. Tested with 1 and 6 disks on ubuntu 15.04 hvm, windows 7 and windows 8 domUs. Doc entry and libxl.h define should be added, I'll do. Other emulated disks cases should be converted to use new qemu parameters but probably a fix in qemu is needed. Any comment is appreciated. Signed-off-by: Fabio Fantoni Changes in v2: - libxl_dm.c: manual bus and unit selection (as workaround to qemu bug) to have multiple disks working. --- tools/libxl/libxl_create.c | 1 + tools/libxl/libxl_dm.c | 10 +- tools/libxl/libxl_types.idl | 1 + tools/libxl/xl_cmdimpl.c| 1 + 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index f0da7dc..fcfe24a 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -322,6 +322,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm, false); libxl_defbool_setdefault(&b_info->u.hvm.usb,false); libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true); +libxl_defbool_setdefault(&b_info->u.hvm.ahci, false); libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false); if (!libxl_defbool_val(b_info->u.hvm.spice.enable) && diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 0c6408d..4bec5ba 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -804,6 +804,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size)); if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) { +if (libxl_defbool_val(b_info->u.hvm.ahci)) +flexarray_append_pair(dm_args, "-device", "ahci,id=ahci0"); for (i = 0; i < num_disks; i++) { int disk, part; int dev_number = @@ -858,7 +860,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, drive = libxl__sprintf (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback", pdev_path, disk, format); -else if (disk < 4) +else if (disk < 6 && libxl_defbool_val(b_info->u.hvm.ahci)){ +flexarray_vappend(dm_args, "-drive", + GCSPRINTF("file=%s,if=none,id=ahcidisk-%d,format=%s,cache=writeback", +pdev_path, disk, format), "-device", GCSPRINTF("ide-hd,bus=ahci0.%d,unit=0,drive=ahcidisk-%d", +disk, disk), NULL); +continue; +}else if (disk < 4) drive = libxl__sprintf (gc, "file=%s,if=ide,index=%d,media=disk,format=%s,cache=writeback", pdev_path, disk, format); diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 023b21e..6326429 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -433,6 +433,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("nested_hvm", libxl_defbool), ("smbios_firmware", string), ("acpi_firmware",string), + ("ahci", libxl_defbool), ("nographic",libxl_defbool), ("vga", libxl_vga_interface_info), ("vnc", libxl_vnc_info), diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 526a1f6..9475a16 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -2195,6 +2195,7 @@ skip_vfb: xlu_cfg_replace_string (config, "soundhw", &b_info->u.hvm.soundhw, 0); xlu_cfg_get_defbool(config, "xen_platform_pci", &b_info->u.hvm.xen_platform_pci, 0); +xlu_cfg_get_defbool(config, "ahci", &b_info->u.hvm.ahci, 0); if(b_info->u.hvm.vnc.listen && b_info->u.hvm.vnc.display -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Wed, 13 May 2015, John Snow wrote: > On 05/13/2015 02:15 PM, Stefano Stabellini wrote: > > On Wed, 13 May 2015, Daniel P. Berrange wrote: > >> On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: > >>> Do not emulate a floppy drive if no drives are supposed to be present. > >>> > >>> This fixes the behavior of -nodefaults, that should remove the floppy > >>> drive (see docs/qdev-device-use.txt:Default Devices), but actually > >>> doesn't. > >> > >> Technically that doc is just refering to the disablement of the > >> primary floppy drive, as opposed to the floppy controller. The > >> floppy controller itself is really a built-in device that is > >> defined as part of the machine type, along with the various other > >> platform devices hanging off the PIIX and ISA brige. > > > > I think you are right, this patch is a bit too harsh in fixing the > > problem: I just wanted to properly disable drive emulation, because from > > my tests the guest thinks that one drive is present even when is not. > > > > We should just be a little careful in explaining the difference between > the controller, the drives, and what circumstances things show up and to > whom. > > Currently the FDC is always present and always has two drive objects > that are directly inlined to that device. > > Now, based on whether or not this line fires in vl.c: > default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS); > > controls whether or not we find that drive in pc_basic_device_init as > you've found, which controls (ultimately) whether or not > fdctrl->drives[0].blk or fdctrl->drives[1].blk gets set. > > If the blk pointer is set, even if you have no media inserted, > fd_revalidate (and pick_geometry) will *FORCIBLY PICK* a drive type for > this floppy, currently a 1.44MB type, (20 sect, 80 track, 1 head) with a > data rate of 500 Kbps. This is written to the rtc memory where seabios > later reads it to discover if you have a guest-visible floppy drive there. > > Both QEMU and SeaBIOS confuse the concept of "A drive is present" with > "A disk is present" which leads to these kinds of confusing scenarios. > > There's some work to do here, for sure. That was my impression too. On Thu, 14 May 2015, Stefan Weil wrote: > Am 13.05.2015 um 20:15 schrieb Stefano Stabellini: > > On Wed, 13 May 2015, Daniel P. Berrange wrote: > > > On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: > > > > Do not emulate a floppy drive if no drives are supposed to be present. > > > > > > > > This fixes the behavior of -nodefaults, that should remove the floppy > > > > drive (see docs/qdev-device-use.txt:Default Devices), but actually > > > > doesn't. > > > Technically that doc is just refering to the disablement of the > > > primary floppy drive, as opposed to the floppy controller. The > > > floppy controller itself is really a built-in device that is > > > defined as part of the machine type, along with the various other > > > platform devices hanging off the PIIX and ISA brige. > > I think you are right, this patch is a bit too harsh in fixing the > > problem: I just wanted to properly disable drive emulation, because from > > my tests the guest thinks that one drive is present even when is not. > > A short test on some of my physical machines shows that most > of them don't have a floppy disk controller at all (dmesg | grep FDC). > > Only some older machines still have one. > > Therefore I think that QEMU must also be able to offer a virtual > machine without an FDC, maybe as the default for the next > version of QEMU. I think it would make sense to make it the default for -nodefaults machines. I would be OK with a new property too, as we could set it from libxl or libvirt. Anybody would be happy to pick this one up or should I do it? ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 04/14] libxc/migration: Pass checkpoint information into the save algorithm.
On Thu, 2015-05-14 at 09:03 +0800, Yang Hongyang wrote: > > On 05/13/2015 11:49 PM, Ian Campbell wrote: > > On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote: > >> From: Andrew Cooper > > > > OOI how was this signalled to the old code? > > The old code check the callbacks "postcopy & checkpoint", > if the callbacks exists, it will call them which I think is > unreliable, so I add this flag to explicitly indicate a > checkpointed stream in the new code. However, it is backward > compatible, the legacy migration just don't know this flag > and will ignore it. That makes sense, thanks. I think it would be good to quickly mention this in the commit log. If there is no other reason to make a v7 then you can just reply to v6 with some text and I'll merge it in on commit. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams
On Thu, 2015-05-14 at 09:52 +0800, Yang Hongyang wrote: > > On 05/13/2015 11:56 PM, Ian Campbell wrote: > > On Wed, 2015-05-13 at 16:47 +0100, Andrew Cooper wrote: > >> On 13/05/15 16:46, Ian Campbell wrote: > >>> On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote: > From: Andrew Cooper > > There are some records which should only be sent once in the stream, and > not > repeated for each checkpoint. {start,end}_of_stream() become > per-checkpoint, > and a new start_of_stream() is introduced. > > There is no resulting change record order, but the X86_PV_INFO record is > identified as once per stream. Currently the X86_PV_P2M_FRAMES record > is as > well, but this is because of an implementation bug and can move back to > being > on an as-needed basis when fixed. > > In addition, a few minor adjustments of comments and layout. > > Signed-off-by: Andrew Cooper > >>> Subject to my finding a suitable documentation update further down the > >>> series: > >>> > >>> Acked-by: Ian Campbell > >> > >> What update are you expecting? X86_PV_INFO is already strictly defined > >> as "sent once" in the spec. > > > > Something defining a checkpointed stread. > > I think this is done in the 3rd patch? Can I take this ack? Yes, thanks. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Thu, May 14, 2015 at 12:12:52PM +0100, Stefano Stabellini wrote: > On Wed, 13 May 2015, John Snow wrote: > > On 05/13/2015 02:15 PM, Stefano Stabellini wrote: > > > On Wed, 13 May 2015, Daniel P. Berrange wrote: > > >> On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: > > >>> Do not emulate a floppy drive if no drives are supposed to be present. > > >>> > > >>> This fixes the behavior of -nodefaults, that should remove the floppy > > >>> drive (see docs/qdev-device-use.txt:Default Devices), but actually > > >>> doesn't. > > >> > > >> Technically that doc is just refering to the disablement of the > > >> primary floppy drive, as opposed to the floppy controller. The > > >> floppy controller itself is really a built-in device that is > > >> defined as part of the machine type, along with the various other > > >> platform devices hanging off the PIIX and ISA brige. > > > > > > I think you are right, this patch is a bit too harsh in fixing the > > > problem: I just wanted to properly disable drive emulation, because from > > > my tests the guest thinks that one drive is present even when is not. > > > > > > > We should just be a little careful in explaining the difference between > > the controller, the drives, and what circumstances things show up and to > > whom. > > > > Currently the FDC is always present and always has two drive objects > > that are directly inlined to that device. > > > > Now, based on whether or not this line fires in vl.c: > > default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS); > > > > controls whether or not we find that drive in pc_basic_device_init as > > you've found, which controls (ultimately) whether or not > > fdctrl->drives[0].blk or fdctrl->drives[1].blk gets set. > > > > If the blk pointer is set, even if you have no media inserted, > > fd_revalidate (and pick_geometry) will *FORCIBLY PICK* a drive type for > > this floppy, currently a 1.44MB type, (20 sect, 80 track, 1 head) with a > > data rate of 500 Kbps. This is written to the rtc memory where seabios > > later reads it to discover if you have a guest-visible floppy drive there. > > > > Both QEMU and SeaBIOS confuse the concept of "A drive is present" with > > "A disk is present" which leads to these kinds of confusing scenarios. > > > > There's some work to do here, for sure. > > That was my impression too. > > > > On Thu, 14 May 2015, Stefan Weil wrote: > > Am 13.05.2015 um 20:15 schrieb Stefano Stabellini: > > > On Wed, 13 May 2015, Daniel P. Berrange wrote: > > > > On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: > > > > > Do not emulate a floppy drive if no drives are supposed to be present. > > > > > > > > > > This fixes the behavior of -nodefaults, that should remove the floppy > > > > > drive (see docs/qdev-device-use.txt:Default Devices), but actually > > > > > doesn't. > > > > Technically that doc is just refering to the disablement of the > > > > primary floppy drive, as opposed to the floppy controller. The > > > > floppy controller itself is really a built-in device that is > > > > defined as part of the machine type, along with the various other > > > > platform devices hanging off the PIIX and ISA brige. > > > I think you are right, this patch is a bit too harsh in fixing the > > > problem: I just wanted to properly disable drive emulation, because from > > > my tests the guest thinks that one drive is present even when is not. > > > > A short test on some of my physical machines shows that most > > of them don't have a floppy disk controller at all (dmesg | grep FDC). > > > > Only some older machines still have one. > > > > Therefore I think that QEMU must also be able to offer a virtual > > machine without an FDC, maybe as the default for the next > > version of QEMU. > > I think it would make sense to make it the default for -nodefaults > machines. I would be OK with a new property too, as we could set it from > libxl or libvirt. Anybody would be happy to pick this one up or should I > do it? It isn't permissible to change the hardware exposed to guests for existing machine types, even when -nodefaults is used. Any change in defaults has to be for new machine types only. eg we can't change pc-2.3.0 machine type, but we could change defaults for the pc-2.4.0 machine type that will be in next release. That ensures migration upgrade compatibility for existing guests, while letting new guests get the new defaults. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 02/16] libxc/save: Adjust stream-position callbacks for checkpointed streams
On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > From: Andrew Cooper > > There are some records which should only be sent once in the stream, and not > repeated for each checkpoint. {start,end}_of_stream() become per-checkpoint, > and a new start_of_stream() is introduced. > > There is no resulting change record order, but the X86_PV_INFO record is > identified as once per stream. Currently the X86_PV_P2M_FRAMES record is as > well, but this is because of an implementation bug and can move back to being > on an as-needed basis when fixed. > > In addition, a few minor adjustments of comments and layout. > > Signed-off-by: Andrew Cooper You pointed me to the docs in v5, so my previous Acked-by: Ian Campbell stands. (just replying here so I don't have to check two threads when I come to reply). ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCHv6 1/3] xen: use ticket locks for spin locks
Replace the byte locks with ticket locks. Ticket locks are: a) fair; and b) peform better when contented since they spin without an atomic operation. The lock is split into two ticket values: head and tail. A locker acquires a ticket by (atomically) increasing tail and using the previous tail value. A CPU holds the lock if its ticket == head. The lock is released by increasing head. spin_lock_irq() and spin_lock_irqsave() now spin with irqs disabled (previously, they would spin with irqs enabled if possible). This is required to prevent deadlocks when the irq handler tries to take the same lock with a higher ticket. Architectures need only provide arch_fetch_and_add() and two barriers: arch_lock_acquire_barrier() and arch_lock_release_barrier(). Signed-off-by: David Vrabel --- xen/common/spinlock.c| 116 -- xen/include/asm-arm/system.h |3 ++ xen/include/asm-x86/system.h | 11 xen/include/xen/spinlock.h | 16 -- 4 files changed, 92 insertions(+), 54 deletions(-) diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c index 5fd8b1c..c8dc8ba 100644 --- a/xen/common/spinlock.c +++ b/xen/common/spinlock.c @@ -115,125 +115,134 @@ void spin_debug_disable(void) #endif +static always_inline spinlock_tickets_t observe_lock(spinlock_tickets_t *t) +{ +spinlock_tickets_t v; + +smp_rmb(); +v.head_tail = read_atomic(&t->head_tail); +return v; +} + +static always_inline u16 observe_head(spinlock_tickets_t *t) +{ +smp_rmb(); +return read_atomic(&t->head); +} + void _spin_lock(spinlock_t *lock) { +spinlock_tickets_t tickets = { .tail = 1, }; LOCK_PROFILE_VAR; check_lock(&lock->debug); -while ( unlikely(!_raw_spin_trylock(&lock->raw)) ) +tickets.head_tail = arch_fetch_and_add(&lock->tickets.head_tail, + tickets.head_tail); +while ( tickets.tail != observe_head(&lock->tickets) ) { LOCK_PROFILE_BLOCK; -while ( likely(_raw_spin_is_locked(&lock->raw)) ) -cpu_relax(); +cpu_relax(); } LOCK_PROFILE_GOT; preempt_disable(); +arch_lock_acquire_barrier(); } void _spin_lock_irq(spinlock_t *lock) { -LOCK_PROFILE_VAR; - ASSERT(local_irq_is_enabled()); local_irq_disable(); -check_lock(&lock->debug); -while ( unlikely(!_raw_spin_trylock(&lock->raw)) ) -{ -LOCK_PROFILE_BLOCK; -local_irq_enable(); -while ( likely(_raw_spin_is_locked(&lock->raw)) ) -cpu_relax(); -local_irq_disable(); -} -LOCK_PROFILE_GOT; -preempt_disable(); +_spin_lock(lock); } unsigned long _spin_lock_irqsave(spinlock_t *lock) { unsigned long flags; -LOCK_PROFILE_VAR; local_irq_save(flags); -check_lock(&lock->debug); -while ( unlikely(!_raw_spin_trylock(&lock->raw)) ) -{ -LOCK_PROFILE_BLOCK; -local_irq_restore(flags); -while ( likely(_raw_spin_is_locked(&lock->raw)) ) -cpu_relax(); -local_irq_disable(); -} -LOCK_PROFILE_GOT; -preempt_disable(); +_spin_lock(lock); return flags; } void _spin_unlock(spinlock_t *lock) { +arch_lock_release_barrier(); preempt_enable(); LOCK_PROFILE_REL; -_raw_spin_unlock(&lock->raw); +add_sized(&lock->tickets.head, 1); } void _spin_unlock_irq(spinlock_t *lock) { -preempt_enable(); -LOCK_PROFILE_REL; -_raw_spin_unlock(&lock->raw); +_spin_unlock(lock); local_irq_enable(); } void _spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags) { -preempt_enable(); -LOCK_PROFILE_REL; -_raw_spin_unlock(&lock->raw); +_spin_unlock(lock); local_irq_restore(flags); } int _spin_is_locked(spinlock_t *lock) { check_lock(&lock->debug); -return _raw_spin_is_locked(&lock->raw); +return lock->tickets.head != lock->tickets.tail; } int _spin_trylock(spinlock_t *lock) { +spinlock_tickets_t old, new; + check_lock(&lock->debug); -if ( !_raw_spin_trylock(&lock->raw) ) +old = observe_lock(&lock->tickets); +if ( old.head != old.tail ) +return 0; +new = old; +new.tail++; +if ( cmpxchg(&lock->tickets.head_tail, + old.head_tail, new.head_tail) != old.head_tail ) return 0; #ifdef LOCK_PROFILE if (lock->profile) lock->profile->time_locked = NOW(); #endif preempt_disable(); +/* + * cmpxchg() is a full barrier so no need for an + * arch_lock_acquire_barrier(). + */ return 1; } void _spin_barrier(spinlock_t *lock) { +spinlock_tickets_t sample; #ifdef LOCK_PROFILE s_time_t block = NOW(); -u64 loop = 0; +#endif check_barrier(&lock->debug); -do { smp_mb(); loop++;} while ( _raw_spin_is_locked(&lock->raw) ); -if ((loop > 1) && lock->profile) +smp_mb(); +sample = observe_lock(&lock->tickets); +if ( samp
[Xen-devel] [PATCHv6 0/3] Use ticket locks for spinlocks
Use ticket locks for spin locks instead of the current byte locks. Ticket locks are fair. This is an important property for hypervisor locks. Note that spin_lock_irq() and spin_lock_irqsave() now spin with irqs disabled (previously, they would spin with irqs enabled if possible). This is required to prevent deadlocks when the irq handler tries to take the same lock with a higher ticket. We have analysed the affect of this series on interrupt latency (by measuring the duration of irq disable/enable regions) and there is no signficant impact. http://xenbits.xen.org/people/dvrabel/ticket-locks/busy_comp.png Thanks to Jennifer Herbert for performing this analysis. Performance results (using a earlier prototype) of aggregate network throughput between 20 VIF pairs shows good improvements. http://xenbits.xen.org/people/dvrabel/3336.png This benchmark is limited by contention on the map track lock. Changes in v6: - Check sample.head in _spin_barrier(). - Improve comments. Changes in v5: - _spin_barrier() fixes (re-add smp_mb(), profiling, cpu_relax()). - Cleanup x86 add_sized(). Changes in v4: - Use new add_sized() to increment ticket head. - Add arch_lock_acquire_barrier() and arch_lock_release_barrier(). - Rename xadd() to arch_fetch_and_add(). - Shrink struct domain by 16 bytes. Changes in v3: - xadd() implementation for arm32 and arm64. - Add barriers required on arm. - Only wait for the current lock holder in _spin_barrier(). Changes in v2: - Reimplemented in C, requiring only an arch-specific xadd() op - Optimize spin_lock_recursive() to call spin_lock() since trylock loops are bad with ticket locks. David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCHv6 2/3] x86, arm: remove asm/spinlock.h from all architectures
Now that all architecture use a common ticket lock implementation for spinlocks, remove the architecture specific byte lock implementations. Signed-off-by: David Vrabel Reviewed-by: Tim Deegan Acked-by: Jan Beulich Acked-by: Ian Campbell --- xen/arch/arm/README.LinuxPrimitives | 28 --- xen/include/asm-arm/arm32/spinlock.h | 66 -- xen/include/asm-arm/arm64/spinlock.h | 63 xen/include/asm-arm/spinlock.h | 23 xen/include/asm-x86/spinlock.h | 37 --- xen/include/xen/spinlock.h |1 - 6 files changed, 218 deletions(-) delete mode 100644 xen/include/asm-arm/arm32/spinlock.h delete mode 100644 xen/include/asm-arm/arm64/spinlock.h delete mode 100644 xen/include/asm-arm/spinlock.h delete mode 100644 xen/include/asm-x86/spinlock.h diff --git a/xen/arch/arm/README.LinuxPrimitives b/xen/arch/arm/README.LinuxPrimitives index 7f33fc7..3115f51 100644 --- a/xen/arch/arm/README.LinuxPrimitives +++ b/xen/arch/arm/README.LinuxPrimitives @@ -25,16 +25,6 @@ linux/arch/arm64/include/asm/atomic.h xen/include/asm-arm/arm64/atomic.h - -spinlocks: last sync @ v3.16-rc6 (last commit: 95c4189689f9) - -linux/arch/arm64/include/asm/spinlock.h xen/include/asm-arm/arm64/spinlock.h - -Skipped: - 5686b06 arm64: lockref: add support for lockless lockrefs using cmpxchg - 52ea2a5 arm64: locks: introduce ticket-based spinlock implementation - -- - mem*: last sync @ v3.16-rc6 (last commit: d875c9b37240) linux/arch/arm64/lib/memchr.S xen/arch/arm/arm64/lib/memchr.S @@ -103,24 +93,6 @@ linux/arch/arm/include/asm/atomic.h xen/include/asm-arm/arm32/atomic.h - -spinlocks: last sync: 15e7e5c1ebf5 - -linux/arch/arm/include/asm/spinlock.h xen/include/asm-arm/arm32/spinlock.h - -*** Linux has switched to ticket locks but we still use bitlocks. - -resync to v3.14-rc7: - - 7c8746a ARM: 7955/1: spinlock: ensure we have a compiler barrier before sev - 0cbad9c ARM: 7854/1: lockref: add support for lockless lockrefs using cmpxchg64 - 9bb17be ARM: locks: prefetch the destination word for write prior to strex - 27a8479 ARM: smp_on_up: move inline asm ALT_SMP patching macro out of spinlock. - 00efaa0 ARM: 7812/1: rwlocks: retry trylock operation if strex fails on free lo - afa31d8 ARM: 7811/1: locks: use early clobber in arch_spin_trylock - 73a6fdc ARM: spinlock: use inner-shareable dsb variant prior to sev instruction - -- - mem*: last sync @ v3.16-rc6 (last commit: d98b90ea22b0) linux/arch/arm/lib/copy_template.S xen/arch/arm/arm32/lib/copy_template.S diff --git a/xen/include/asm-arm/arm32/spinlock.h b/xen/include/asm-arm/arm32/spinlock.h deleted file mode 100644 index bc0343c..000 --- a/xen/include/asm-arm/arm32/spinlock.h +++ /dev/null @@ -1,66 +0,0 @@ -#ifndef __ASM_ARM32_SPINLOCK_H -#define __ASM_ARM32_SPINLOCK_H - -static inline void dsb_sev(void) -{ -__asm__ __volatile__ ( -"dsb\n" -"sev\n" -); -} - -typedef struct { -volatile unsigned int lock; -} raw_spinlock_t; - -#define _RAW_SPIN_LOCK_UNLOCKED { 0 } - -#define _raw_spin_is_locked(x) ((x)->lock != 0) - -static always_inline void _raw_spin_unlock(raw_spinlock_t *lock) -{ -ASSERT(_raw_spin_is_locked(lock)); - -smp_mb(); - -__asm__ __volatile__( -" str %1, [%0]\n" -: -: "r" (&lock->lock), "r" (0) -: "cc"); - -dsb_sev(); -} - -static always_inline int _raw_spin_trylock(raw_spinlock_t *lock) -{ -unsigned long contended, res; - -do { -__asm__ __volatile__( -" ldrex %0, [%2]\n" -" teq %0, #0\n" -" strexeq %1, %3, [%2]\n" -" movne %1, #0\n" -: "=&r" (contended), "=r" (res) -: "r" (&lock->lock), "r" (1) -: "cc"); -} while (res); - -if (!contended) { -smp_mb(); -return 1; -} else { -return 0; -} -} - -#endif /* __ASM_SPINLOCK_H */ -/* - * Local variables: - * mode: C - * c-file-style: "BSD" - * c-basic-offset: 4 - * indent-tabs-mode: nil - * End: - */ diff --git a/xen/include/asm-arm/arm64/spinlock.h b/xen/include/asm-arm/arm64/spinlock.h deleted file mode 100644 index 5ae034d..000 --- a/xen/include/asm-arm/arm64/spinlock.h +++ /dev/null @@ -1,63 +0,0 @@ -/* - * Derived from Linux arch64 spinlock.h which is: - * Copyright (C) 2012 ARM Ltd. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without ev
[Xen-devel] [PATCHv6 3/3] x86: reduce struct hvm_domain size
Pack struct hvm_domain to reduce it by 8 bytes. Thus reducing the size of struct domain by 8 bytes. Signed-off-by: David Vrabel --- xen/include/asm-x86/hvm/domain.h | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h index 0f8b19a..fb30903 100644 --- a/xen/include/asm-x86/hvm/domain.h +++ b/xen/include/asm-x86/hvm/domain.h @@ -115,12 +115,6 @@ struct hvm_domain { /* VRAM dirty support. Protect with the domain paging lock. */ struct sh_dirty_vram *dirty_vram; -/* If one of vcpus of this domain is in no_fill_mode or - * mtrr/pat between vcpus is not the same, set is_in_uc_mode - */ -spinlock_t uc_lock; -bool_t is_in_uc_mode; - /* Pass-through */ struct hvm_iommu hvm_iommu; @@ -135,6 +129,12 @@ struct hvm_domain { bool_t qemu_mapcache_invalidate; bool_t is_s3_suspended; +/* If one of vcpus of this domain is in no_fill_mode or + * mtrr/pat between vcpus is not the same, set is_in_uc_mode + */ +bool_t is_in_uc_mode; +spinlock_t uc_lock; + /* * TSC value that VCPUs use to calculate their tsc_offset value. * Used during initialization and save/restore. -- 1.7.10.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 04/16] libxc/migration: Pass checkpoint information into the save algorithm.
On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > From: Andrew Cooper > > Signed-off-by: Andrew Cooper As I mentioned in v5 (after you sent v6) please propose a few words about how this works with the v1 code and then: Acked-by: Ian Campbell > CC: Ian Jackson > CC: Wei Liu > CC: Yang Hongyang > --- > tools/libxc/include/xenguest.h | 1 + > tools/libxc/xc_sr_common.h | 3 +++ > tools/libxc/xc_sr_save.c | 3 +++ > tools/libxl/libxl_dom.c| 1 + > 4 files changed, 8 insertions(+) > > diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h > index 8e39075..7581263 100644 > --- a/tools/libxc/include/xenguest.h > +++ b/tools/libxc/include/xenguest.h > @@ -30,6 +30,7 @@ > #define XCFLAGS_HVM (1 << 2) > #define XCFLAGS_STDVGA(1 << 3) > #define XCFLAGS_CHECKPOINT_COMPRESS(1 << 4) > +#define XCFLAGS_CHECKPOINTED(1 << 5) > > #define X86_64_B_SIZE 64 > #define X86_32_B_SIZE 32 > diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h > index c4fe92c..c0f90d4 100644 > --- a/tools/libxc/xc_sr_common.h > +++ b/tools/libxc/xc_sr_common.h > @@ -174,6 +174,9 @@ struct xc_sr_context > /* Live migrate vs non live suspend. */ > bool live; > > +/* Plain VM, or checkpoints over time. */ > +bool checkpointed; > + > /* Further debugging information in the stream. */ > bool debug; > > diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c > index 66fcd3e..caa727d 100644 > --- a/tools/libxc/xc_sr_save.c > +++ b/tools/libxc/xc_sr_save.c > @@ -732,6 +732,7 @@ int xc_domain_save2(xc_interface *xch, int io_fd, > uint32_t dom, > ctx.save.callbacks = callbacks; > ctx.save.live = !!(flags & XCFLAGS_LIVE); > ctx.save.debug = !!(flags & XCFLAGS_DEBUG); > +ctx.save.checkpointed = !!(flags & XCFLAGS_CHECKPOINTED); > > /* > * TODO: Find some time to better tweak the live migration algorithm. > @@ -745,6 +746,8 @@ int xc_domain_save2(xc_interface *xch, int io_fd, > uint32_t dom, > /* Sanity checks for callbacks. */ > if ( hvm ) > assert(callbacks->switch_qemu_logdirty); > +if ( ctx.save.checkpointed ) > +assert(callbacks->checkpoint && callbacks->postcopy); > > IPRINTF("In experimental %s", __func__); > DPRINTF("fd %d, dom %u, max_iters %u, max_factor %u, flags %u, hvm %d", > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index f408646..a0c9850 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -2003,6 +2003,7 @@ void libxl__domain_suspend(libxl__egc *egc, > libxl__domain_suspend_state *dss) > > if (r_info != NULL) { > dss->interval = r_info->interval; > +dss->xcflags |= XCFLAGS_CHECKPOINTED; > if (libxl_defbool_val(r_info->compression)) > dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS; > } ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 05/16] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW
On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > There are cases where we only need to use the hypercall buffer data, > and do not use the xc_hypercall_buffer_t struct. > DECLARE_HYPERCALL_BUFFER_SHADOW defines a user pointer that can allow > us to access the hypercall buffer data but it also defines a > xc_hypercall_buffer_t that we don't use, the compiler will report arg > unused error. > Add __attribute__((unused)) before xc_hypercall_buffer_t to avoid > the compiler error. > > Example cases: > In send_all_pages(), we only need to use the hypercall buffer data > which is a dirty bitmap, we set the dirty bitmap to all dirty and call > send_dirty_pages, we will not use the xc_hypercall_buffer_t and hypercall > to retrieve the dirty bitmap. > In send_some_pages(), we will also only need to use the dirty_bitmap. > the retrieve dirty bitmap hypercall are done by the caller. > > Signed-off-by: Yang Hongyang Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: assigned a default ssid_label (XSM label) to guests
Hi Ian, On 14/05/15 11:33, Ian Campbell wrote: > system_u:system_r:domU_t is defined in the default policy and makes as > much sense as anything for a default. So you rule out the possibility to run an unlabelled domain? This is possible if the policy explicitly authorized it. That's a significant change in the libxl behavior. IHMO, having a default policy doesn't mean libxl should set a default ssid to make XSM transparent to the user. The explicit ssid makes clear that the guest is using a ssid foo and if it's not provided then it will fail to boot. Setting a default value may hide a bigger issue and take the wrong policy the user forgot to set up an ssid. > This change required moving the call to domain_create_info_setdefault > to be before the ssid_label is translated into ssidref, which also > moves it before some other stuff which consumes things from c_info, > which is correct since setdefault should always be called first. Apart > from the SSID handling there should be no functional change (since > setdefault doesn't actually act on anything which that other stuff > uses). > > There is no need to set exec_ssid_label since the default is to leave > the domain using the ssid_label after build. By setting a ssid label, libxl will print a new warning on Xen not built with XSM which will confuse the user: libxl: warning: libxl_create.c:813:initiate_domain_create: XSM Disabled: init_seclabel not supported > > I haven't done anything with the device model ssid. > > Signed-off-by: Ian Campbell > Cc: Daniel De Graaf > Cc: wei.l...@citrix.com > --- > docs/man/xl.cfg.pod.5 |4 +++- > tools/libxl/libxl_create.c | 11 --- > 2 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 > index 8e4154f..fcca1cc 100644 > --- a/docs/man/xl.cfg.pod.5 > +++ b/docs/man/xl.cfg.pod.5 > @@ -437,7 +437,9 @@ UUID will be generated. > > =item B > > -Assign an XSM security label to this domain. > +Assign an XSM security label to this domain. By default a domain is > +assigned the label B, which is defined in > +the default policy. It's not easy to know that seclabel will be stored in ssid_label. It would be good to have this explanation into the toolstack API. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv4 5/5] x86: reduce struct hvm_domain size
At 15:37 +0100 on 11 May (1431358625), David Vrabel wrote: > Pack struct hvm_domain to reduce it by 8 bytes. Thus reducing the > size of struct domain by 8 bytes. In my builds (non-debug, on current staging), this makes no difference to struct domain. struct hvm_domain gets 8 bytes smaller (2144 -> 2136), but struct arch_domain remains at 2944 bytes (it just gets an extra 8 bytes in the padding at the end to round it up to __cacheline_aligned). Tim. > Signed-off-by: David Vrabel > --- > xen/include/asm-x86/hvm/domain.h | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/xen/include/asm-x86/hvm/domain.h > b/xen/include/asm-x86/hvm/domain.h > index 0f8b19a..fb30903 100644 > --- a/xen/include/asm-x86/hvm/domain.h > +++ b/xen/include/asm-x86/hvm/domain.h > @@ -115,12 +115,6 @@ struct hvm_domain { > /* VRAM dirty support. Protect with the domain paging lock. */ > struct sh_dirty_vram *dirty_vram; > > -/* If one of vcpus of this domain is in no_fill_mode or > - * mtrr/pat between vcpus is not the same, set is_in_uc_mode > - */ > -spinlock_t uc_lock; > -bool_t is_in_uc_mode; > - > /* Pass-through */ > struct hvm_iommu hvm_iommu; > > @@ -135,6 +129,12 @@ struct hvm_domain { > bool_t qemu_mapcache_invalidate; > bool_t is_s3_suspended; > > +/* If one of vcpus of this domain is in no_fill_mode or > + * mtrr/pat between vcpus is not the same, set is_in_uc_mode > + */ > +bool_t is_in_uc_mode; > +spinlock_t uc_lock; > + > /* > * TSC value that VCPUs use to calculate their tsc_offset value. > * Used during initialization and save/restore. > -- > 1.7.10.4 > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 06/16] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro
On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > When we use a DECLARE_HYPERCALL_BUFFER_SHADOW define a user ^it defines > pointer '_name' and a shadow xc_hypercall_buffer_t. > then call xc_hypercall_buffer_free_pages(_xch, _name, _nr), ^When then calling > the complier will report '_name' unused error, it's because ^drop it's > xc_hypercall_buffer_free_pages() is a MACRO and '_name' ^is > transparently converted to the hypercall buffer. it confused confuses > the caller because xc_hypercall_buffer_free_pages() do look looks > like a function and take '_name' as an arg. takes > Add an if check to let the compiler think we are actually > using the argument '_name'. > > Signed-off-by: Yang Hongyang I can fix those on commit if there is no other reason for a v7. Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 08/16] libxc/save: rename to_send to dirty_bitmap
On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > rename to_send to dirty_bitmap. > > Signed-off-by: Yang Hongyang Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 07/16] libxc/save: introduce setup() and cleanup() on save
On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > introduce setup() and cleanup() which subsume the > ctx->save.ops.{setup,cleanup}() calls. > The SHADOW_OP_OFF hypercall also included in the cleanup(). ^is also > > Signed-off-by: Yang Hongyang Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 10/16] libxc/save: remove bitmap param from send_some_pages
On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > In last patch we added dirty bitmap to the save context, > we no longer need to pass this param to send_some_pages. > We can get dirty bitmap from the save context. > 'entries' should stay as it is a useful sanity check. > > Signed-off-by: Yang Hongyang Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 09/16] libxc/save: adjust the memory allocation for migration
On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > Move the memory allocation before the concrete live/nolive save > in order to avoid the free/alloc memory loop when using Remus. > > Signed-off-by: Yang Hongyang Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 11/16] libxc/save: rename send_some_pages to send_dirty_pages
On Thu, 2015-05-14 at 09:59 +0100, Andrew Cooper wrote: > On 14/05/15 09:55, Yang Hongyang wrote: > > rename send_some_pages to send_dirty_pages, no functional change. > > > > Signed-off-by: Yang Hongyang > > CC: Ian Campbell > > CC: Ian Jackson > > CC: Wei Liu > > CC: Andrew Cooper > > Reviewed-by: Andrew Cooper Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 13/16] libxc/restore: introduce process_record()
On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > Move record handle codes into a function process_record(). > It will be used multiple times by Remus. > No functional change. > > Signed-off-by: Yang Hongyang Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6 1/3] xen: use ticket locks for spin locks
At 12:21 +0100 on 14 May (1431606100), David Vrabel wrote: > Replace the byte locks with ticket locks. Ticket locks are: a) fair; > and b) peform better when contented since they spin without an atomic > operation. > > The lock is split into two ticket values: head and tail. A locker > acquires a ticket by (atomically) increasing tail and using the > previous tail value. A CPU holds the lock if its ticket == head. The > lock is released by increasing head. > > spin_lock_irq() and spin_lock_irqsave() now spin with irqs disabled > (previously, they would spin with irqs enabled if possible). This is > required to prevent deadlocks when the irq handler tries to take the > same lock with a higher ticket. > > Architectures need only provide arch_fetch_and_add() and two barriers: > arch_lock_acquire_barrier() and arch_lock_release_barrier(). > > Signed-off-by: David Vrabel Reviewed-by: Tim Deegan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 4/4] xen: rework paging_log_dirty_op to work with hvm guests
At 12:58 +0200 on 14 May (1431608313), Roger Pau Monné wrote: > El 14/05/15 a les 11.53, Tim Deegan ha escrit: > > At 16:25 +0200 on 11 May (1431361528), Roger Pau Monne wrote: > >> When the caller of paging_log_dirty_op is a hvm guest Xen would choke when > >> trying to copy the dirty bitmap to the guest because the paging lock is > >> already held. > >> > >> Fix this by independently mapping each page of the guest bitmap as needed > >> without the paging lock held. > >> > >> Signed-off-by: Roger Pau Monné > > > > This looks mostly correct, but OTOH we now seem to have two nested > > stop-and-retry mechsnisms in one function. I think this would be > > cleaner if here: > > > >> +if ( pages >> (3 + PAGE_SHIFT) != > >> + index_mapped >> (3 + PAGE_SHIFT) ) > >> { > >> -rv = -EFAULT; > >> -goto out; > >> +/* We need to map next page */ > >> +d->arch.paging.preempt.log_dirty.i4 = i4; > >> +d->arch.paging.preempt.log_dirty.i3 = i3; > >> +d->arch.paging.preempt.log_dirty.i2 = i2; > >> +d->arch.paging.preempt.log_dirty.done = pages; > >> +paging_unlock(d); > >> +unmap_dirty_bitmap(dirty_bitmap, page); > > > > we set the rest of the preempt state, like so: > > > > d->arch.paging.preempt.dom = current->domain; > > d->arch.paging.preempt.op = sc->op; > > resuming = 1; > > > > and bailed to the very top of the function. I think we might also > > need a hypercall_preempt_check() here as well, so that this restart > > path doesn't stop us ever reaching the preempt_checks below. > > OK, I don't mind adding a preempt check here, although I think we would > hit the ones below anyway as we go scanning the dirty bitmap even if we > have to restart. Oh, so we do. No need for the extra check here, then. But I'd still prefer restarting the whole function to having a second restart point. Cheers, Tim. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Thu, May 14, 2015 at 12:18:26PM +0100, Daniel P. Berrange wrote: > On Thu, May 14, 2015 at 12:12:52PM +0100, Stefano Stabellini wrote: > > On Wed, 13 May 2015, John Snow wrote: > > > On 05/13/2015 02:15 PM, Stefano Stabellini wrote: > > > > On Wed, 13 May 2015, Daniel P. Berrange wrote: > > > >> On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: > > > >>> Do not emulate a floppy drive if no drives are supposed to be present. > > > >>> > > > >>> This fixes the behavior of -nodefaults, that should remove the floppy > > > >>> drive (see docs/qdev-device-use.txt:Default Devices), but actually > > > >>> doesn't. > > > >> > > > >> Technically that doc is just refering to the disablement of the > > > >> primary floppy drive, as opposed to the floppy controller. The > > > >> floppy controller itself is really a built-in device that is > > > >> defined as part of the machine type, along with the various other > > > >> platform devices hanging off the PIIX and ISA brige. > > > > > > > > I think you are right, this patch is a bit too harsh in fixing the > > > > problem: I just wanted to properly disable drive emulation, because from > > > > my tests the guest thinks that one drive is present even when is not. > > > > > > > > > > We should just be a little careful in explaining the difference between > > > the controller, the drives, and what circumstances things show up and to > > > whom. > > > > > > Currently the FDC is always present and always has two drive objects > > > that are directly inlined to that device. > > > > > > Now, based on whether or not this line fires in vl.c: > > > default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS); > > > > > > controls whether or not we find that drive in pc_basic_device_init as > > > you've found, which controls (ultimately) whether or not > > > fdctrl->drives[0].blk or fdctrl->drives[1].blk gets set. > > > > > > If the blk pointer is set, even if you have no media inserted, > > > fd_revalidate (and pick_geometry) will *FORCIBLY PICK* a drive type for > > > this floppy, currently a 1.44MB type, (20 sect, 80 track, 1 head) with a > > > data rate of 500 Kbps. This is written to the rtc memory where seabios > > > later reads it to discover if you have a guest-visible floppy drive there. > > > > > > Both QEMU and SeaBIOS confuse the concept of "A drive is present" with > > > "A disk is present" which leads to these kinds of confusing scenarios. > > > > > > There's some work to do here, for sure. > > > > That was my impression too. > > > > > > > > On Thu, 14 May 2015, Stefan Weil wrote: > > > Am 13.05.2015 um 20:15 schrieb Stefano Stabellini: > > > > On Wed, 13 May 2015, Daniel P. Berrange wrote: > > > > > On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: > > > > > > Do not emulate a floppy drive if no drives are supposed to be > > > > > > present. > > > > > > > > > > > > This fixes the behavior of -nodefaults, that should remove the > > > > > > floppy > > > > > > drive (see docs/qdev-device-use.txt:Default Devices), but actually > > > > > > doesn't. > > > > > Technically that doc is just refering to the disablement of the > > > > > primary floppy drive, as opposed to the floppy controller. The > > > > > floppy controller itself is really a built-in device that is > > > > > defined as part of the machine type, along with the various other > > > > > platform devices hanging off the PIIX and ISA brige. > > > > I think you are right, this patch is a bit too harsh in fixing the > > > > problem: I just wanted to properly disable drive emulation, because from > > > > my tests the guest thinks that one drive is present even when is not. > > > > > > A short test on some of my physical machines shows that most > > > of them don't have a floppy disk controller at all (dmesg | grep FDC). > > > > > > Only some older machines still have one. > > > > > > Therefore I think that QEMU must also be able to offer a virtual > > > machine without an FDC, maybe as the default for the next > > > version of QEMU. > > > > I think it would make sense to make it the default for -nodefaults > > machines. I would be OK with a new property too, as we could set it from > > libxl or libvirt. Anybody would be happy to pick this one up or should I > > do it? > > It isn't permissible to change the hardware exposed to guests for existing > machine types, even when -nodefaults is used. Any change in defaults has > to be for new machine types only. eg we can't change pc-2.3.0 machine > type, but we could change defaults for the pc-2.4.0 machine type that > will be in next release. That ensures migration upgrade compatibility > for existing guests, while letting new guests get the new defaults. > > Regards, > Daniel For libvirt, yes. But command-line users might rely on the old behaviour with e.g. -M pc so we shouldn't change defaults lightly even for new machine types. > -- > |: http://berrange.com -o-http://www.flickr.com/photos/dberr
Re: [Xen-devel] [PATCH v6 12/16] libxc/save: reuse send_dirty_pages() in send_all_pages()
On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > introduce bitmap_set() to set the entire bitmap. > in send_all_pages(), set the entire bitmap and call send_dirty_pages(). > > Signed-off-by: Yang Hongyang Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Thu, May 14, 2015 at 12:12:52PM +0100, Stefano Stabellini wrote: > I would be OK with a new property too, as we could set it from > libxl or libvirt. Anybody would be happy to pick this one up or should I > do it? Pls go ahead, I can merge it in the pc tree. -- MST ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 16/16] libxc/restore: add checkpointed flag to the restore context
On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > add checkpointed flag to the restore context. > > Signed-off-by: Yang Hongyang Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 15/16] libxc/restore: introduce setup() and cleanup() on restore
On Thu, 2015-05-14 at 10:01 +0100, Andrew Cooper wrote: > On 14/05/15 09:55, Yang Hongyang wrote: > > introduce setup() and cleanup() which subsume the > > ctx->restore.ops.{setup,cleanup}() calls and also > > do memory alloc/free. > > > > Signed-off-by: Yang Hongyang > > CC: Andrew Cooper Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 14/16] libxc/restore: split read/handle qemu info
On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > Split read/handle qemu info. The receiving of qemu info > should be done while we receive the migration stream, > handle_qemu will be called when the stream complete. > Otherwise, it will break Remus because read_record() > won't read qemu info and stream_complete will be called > at failover. > > Signed-off-by: Yang Hongyang > CC: Ian Campbell > CC: Ian Jackson > CC: Wei Liu > CC: Andrew Cooper Andy seemed happy with this in v4, so: Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: assigned a default ssid_label (XSM label) to guests
On Thu, 2015-05-14 at 12:21 +0100, Julien Grall wrote: > Hi Ian, > > On 14/05/15 11:33, Ian Campbell wrote: > > system_u:system_r:domU_t is defined in the default policy and makes as > > much sense as anything for a default. > > So you rule out the possibility to run an unlabelled domain? This is > possible if the policy explicitly authorized it. That's a significant > change in the libxl behavior. I didn't realise this was a possibility, wouldn't such a domain be system_u:system_r:unlabeled_t> or something? Note that this won't override a label which is just '' (i.e. an empty string rather than NULL). I don't know if that results in the behaviour you want. When this was discussed before (in a conversation Wei started, but which I can't find, maybe it was IRC rather than email) it seemed that consensus was that by default things should Just Work as if XSM weren't disabled, which is what I've implemented here. > IHMO, having a default policy doesn't mean libxl should set a default > ssid to make XSM transparent to the user. > > The explicit ssid makes clear that the guest is using a ssid foo and if > it's not provided then it will fail to boot. > > Setting a default value may hide a bigger issue and take the wrong > policy the user forgot to set up an ssid. Does domU_t really have so many privileges that this is an issue? I'd expect it to be almost totally privilegeless apart from things which any domU needs. The benefits of XSM seem to mainly apply to the various service domains. Daniel, do you have an opinion here? > > > This change required moving the call to domain_create_info_setdefault > > to be before the ssid_label is translated into ssidref, which also > > moves it before some other stuff which consumes things from c_info, > > which is correct since setdefault should always be called first. Apart > > from the SSID handling there should be no functional change (since > > setdefault doesn't actually act on anything which that other stuff > > uses). > > > > There is no need to set exec_ssid_label since the default is to leave > > the domain using the ssid_label after build. > > By setting a ssid label, libxl will print a new warning on Xen not built > with XSM which will confuse the user: > > libxl: warning: libxl_create.c:813:initiate_domain_create: XSM Disabled: > init_seclabel not supported Ah, I didn't try that case. I'll see if I can work out a way to suppress that warning. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On 14/05/2015 13:47, Michael S. Tsirkin wrote: > > I would be OK with a new property too, as we could set it from > > libxl or libvirt. Anybody would be happy to pick this one up or should I > > do it? > > Pls go ahead, I can merge it in the pc tree. Note that because of the "-drive if=none,id=fdd1 -global isa-fdc.driveA=fdd1" trick to create a floppy drive without if=floppy, the default should remain on---at least for a few releases---even if -nodefaults is passed. Paolo ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Thu, May 14, 2015 at 01:54:22PM +0200, Paolo Bonzini wrote: > > > On 14/05/2015 13:47, Michael S. Tsirkin wrote: > > > I would be OK with a new property too, as we could set it from > > > libxl or libvirt. Anybody would be happy to pick this one up or should I > > > do it? > > > > Pls go ahead, I can merge it in the pc tree. > > Note that because of the "-drive if=none,id=fdd1 -global > isa-fdc.driveA=fdd1" trick to create a floppy drive without if=floppy, > the default should remain on---at least for a few releases---even if > -nodefaults is passed. > > Paolo Exactly. -- MST ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
"Daniel P. Berrange" writes: > On Thu, May 14, 2015 at 12:12:52PM +0100, Stefano Stabellini wrote: >> On Wed, 13 May 2015, John Snow wrote: >> > On 05/13/2015 02:15 PM, Stefano Stabellini wrote: >> > > On Wed, 13 May 2015, Daniel P. Berrange wrote: >> > >> On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: >> > >>> Do not emulate a floppy drive if no drives are supposed to be present. >> > >>> >> > >>> This fixes the behavior of -nodefaults, that should remove the floppy >> > >>> drive (see docs/qdev-device-use.txt:Default Devices), but actually >> > >>> doesn't. >> > >> >> > >> Technically that doc is just refering to the disablement of the >> > >> primary floppy drive, as opposed to the floppy controller. The >> > >> floppy controller itself is really a built-in device that is >> > >> defined as part of the machine type, along with the various other >> > >> platform devices hanging off the PIIX and ISA brige. >> > > >> > > I think you are right, this patch is a bit too harsh in fixing the >> > > problem: I just wanted to properly disable drive emulation, because from >> > > my tests the guest thinks that one drive is present even when is not. >> > > >> > >> > We should just be a little careful in explaining the difference between >> > the controller, the drives, and what circumstances things show up and to >> > whom. >> > >> > Currently the FDC is always present and always has two drive objects >> > that are directly inlined to that device. >> > >> > Now, based on whether or not this line fires in vl.c: >> > default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS); >> > >> > controls whether or not we find that drive in pc_basic_device_init as >> > you've found, which controls (ultimately) whether or not >> > fdctrl->drives[0].blk or fdctrl->drives[1].blk gets set. >> > >> > If the blk pointer is set, even if you have no media inserted, >> > fd_revalidate (and pick_geometry) will *FORCIBLY PICK* a drive type for >> > this floppy, currently a 1.44MB type, (20 sect, 80 track, 1 head) with a >> > data rate of 500 Kbps. This is written to the rtc memory where seabios >> > later reads it to discover if you have a guest-visible floppy drive there. >> > >> > Both QEMU and SeaBIOS confuse the concept of "A drive is present" with >> > "A disk is present" which leads to these kinds of confusing scenarios. >> > >> > There's some work to do here, for sure. >> >> That was my impression too. >> >> >> >> On Thu, 14 May 2015, Stefan Weil wrote: >> > Am 13.05.2015 um 20:15 schrieb Stefano Stabellini: >> > > On Wed, 13 May 2015, Daniel P. Berrange wrote: >> > > > On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: >> > > > > Do not emulate a floppy drive if no drives are supposed to be >> > > > > present. >> > > > > >> > > > > This fixes the behavior of -nodefaults, that should remove the floppy >> > > > > drive (see docs/qdev-device-use.txt:Default Devices), but actually >> > > > > doesn't. >> > > > Technically that doc is just refering to the disablement of the >> > > > primary floppy drive, as opposed to the floppy controller. The >> > > > floppy controller itself is really a built-in device that is >> > > > defined as part of the machine type, along with the various other >> > > > platform devices hanging off the PIIX and ISA brige. >> > > I think you are right, this patch is a bit too harsh in fixing the >> > > problem: I just wanted to properly disable drive emulation, because from >> > > my tests the guest thinks that one drive is present even when is not. >> > >> > A short test on some of my physical machines shows that most >> > of them don't have a floppy disk controller at all (dmesg | grep FDC). >> > >> > Only some older machines still have one. >> > >> > Therefore I think that QEMU must also be able to offer a virtual >> > machine without an FDC, maybe as the default for the next >> > version of QEMU. >> >> I think it would make sense to make it the default for -nodefaults >> machines. I would be OK with a new property too, as we could set it from >> libxl or libvirt. Anybody would be happy to pick this one up or should I >> do it? > > It isn't permissible to change the hardware exposed to guests for existing > machine types, even when -nodefaults is used. Any change in defaults has > to be for new machine types only. eg we can't change pc-2.3.0 machine > type, but we could change defaults for the pc-2.4.0 machine type that > will be in next release. That ensures migration upgrade compatibility > for existing guests, while letting new guests get the new defaults. Correct. Here's how I think it should be done: * Create a machine option to control the FDC This is a machine-specific option. It should only exist for machine types that have an optional FDC. Default must be "on" for old machine types. Default may be "off" for new machine types. It should certainly be off for pc-q35-2.4 and newer. Real Q35 boards commonly don't have an
Re: [Xen-devel] [PATCHv4 5/5] x86: reduce struct hvm_domain size
On 14/05/15 12:24, Tim Deegan wrote: > At 15:37 +0100 on 11 May (1431358625), David Vrabel wrote: >> Pack struct hvm_domain to reduce it by 8 bytes. Thus reducing the >> size of struct domain by 8 bytes. > > In my builds (non-debug, on current staging), this makes no difference > to struct domain. struct hvm_domain gets 8 bytes smaller (2144 -> > 2136), but struct arch_domain remains at 2944 bytes (it just gets an > extra 8 bytes in the padding at the end to round it up to > __cacheline_aligned). It used to make a difference. Perhaps some other recently change negates the benefit? In which case, this patch could be dropped. David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On 14/05/2015 14:02, Markus Armbruster wrote: > It should certainly be off for pc-q35-2.4 and newer. Real Q35 boards > commonly don't have an FDC (depends on the Super I/O chip used). > > We may want to keep it off for pc-i440fx-2.4 and newer. I doubt > there's a real i440FX without an FDC, but our virtual i440FX is quite > unlike a real one in other ways already. That would break libvirt for people upgrading from 2.3 to 2.4. So it's more like pc-i440fx-3.0 and pc-q35-3.0. Unless for q35 we decide to break everything and retroactively nuke the controller. (I'm still not sure why we have backwards-compatible machine types for q35). Paolo > * Create the FDC only if the option is "on". > > * Optional: make -drive if=floppy,... auto-enable it > > I wouldn't bother doing the same for -global isa-fdc.driveA=... and > such. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 04/16] libxc/migration: Pass checkpoint information into the save algorithm.
On Thu, 2015-05-14 at 12:21 +0100, Ian Campbell wrote: > On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > > From: Andrew Cooper > > > > Signed-off-by: Andrew Cooper > > As I mentioned in v5 (after you sent v6) please propose a few words > about how this works with the v1 code and then: > > Acked-by: Ian Campbell Actually since this is the only thing left for this series and timezones suggest you won't be around until tomorrow I'm going to use: The old code checks the callbacks "postcopy & checkpoint", if the callbacks exists, it will call them. However this is unreliable, so add this flag to explicitly indicate a checkpointed stream in the new code. This is backward compatible with the legacy migration just don't know this flag and will ignore it. (which is basically you explanation from v5 reworded slightly to fit a commit log) > > > CC: Ian Jackson > > CC: Wei Liu > > CC: Yang Hongyang > > --- > > tools/libxc/include/xenguest.h | 1 + > > tools/libxc/xc_sr_common.h | 3 +++ > > tools/libxc/xc_sr_save.c | 3 +++ > > tools/libxl/libxl_dom.c| 1 + > > 4 files changed, 8 insertions(+) > > > > diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h > > index 8e39075..7581263 100644 > > --- a/tools/libxc/include/xenguest.h > > +++ b/tools/libxc/include/xenguest.h > > @@ -30,6 +30,7 @@ > > #define XCFLAGS_HVM (1 << 2) > > #define XCFLAGS_STDVGA(1 << 3) > > #define XCFLAGS_CHECKPOINT_COMPRESS(1 << 4) > > +#define XCFLAGS_CHECKPOINTED(1 << 5) > > > > #define X86_64_B_SIZE 64 > > #define X86_32_B_SIZE 32 > > diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h > > index c4fe92c..c0f90d4 100644 > > --- a/tools/libxc/xc_sr_common.h > > +++ b/tools/libxc/xc_sr_common.h > > @@ -174,6 +174,9 @@ struct xc_sr_context > > /* Live migrate vs non live suspend. */ > > bool live; > > > > +/* Plain VM, or checkpoints over time. */ > > +bool checkpointed; > > + > > /* Further debugging information in the stream. */ > > bool debug; > > > > diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c > > index 66fcd3e..caa727d 100644 > > --- a/tools/libxc/xc_sr_save.c > > +++ b/tools/libxc/xc_sr_save.c > > @@ -732,6 +732,7 @@ int xc_domain_save2(xc_interface *xch, int io_fd, > > uint32_t dom, > > ctx.save.callbacks = callbacks; > > ctx.save.live = !!(flags & XCFLAGS_LIVE); > > ctx.save.debug = !!(flags & XCFLAGS_DEBUG); > > +ctx.save.checkpointed = !!(flags & XCFLAGS_CHECKPOINTED); > > > > /* > > * TODO: Find some time to better tweak the live migration algorithm. > > @@ -745,6 +746,8 @@ int xc_domain_save2(xc_interface *xch, int io_fd, > > uint32_t dom, > > /* Sanity checks for callbacks. */ > > if ( hvm ) > > assert(callbacks->switch_qemu_logdirty); > > +if ( ctx.save.checkpointed ) > > +assert(callbacks->checkpoint && callbacks->postcopy); > > > > IPRINTF("In experimental %s", __func__); > > DPRINTF("fd %d, dom %u, max_iters %u, max_factor %u, flags %u, hvm %d", > > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > > index f408646..a0c9850 100644 > > --- a/tools/libxl/libxl_dom.c > > +++ b/tools/libxl/libxl_dom.c > > @@ -2003,6 +2003,7 @@ void libxl__domain_suspend(libxl__egc *egc, > > libxl__domain_suspend_state *dss) > > > > if (r_info != NULL) { > > dss->interval = r_info->interval; > > +dss->xcflags |= XCFLAGS_CHECKPOINTED; > > if (libxl_defbool_val(r_info->compression)) > > dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS; > > } > > > > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: assigned a default ssid_label (XSM label) to guests
On Thu, May 14, 2015 at 11:33:45AM +0100, Ian Campbell wrote: > system_u:system_r:domU_t is defined in the default policy and makes as > much sense as anything for a default. > > This change required moving the call to domain_create_info_setdefault > to be before the ssid_label is translated into ssidref, which also > moves it before some other stuff which consumes things from c_info, > which is correct since setdefault should always be called first. Apart > from the SSID handling there should be no functional change (since > setdefault doesn't actually act on anything which that other stuff > uses). > > There is no need to set exec_ssid_label since the default is to leave > the domain using the ssid_label after build. > > I haven't done anything with the device model ssid. > > Signed-off-by: Ian Campbell > Cc: Daniel De Graaf > Cc: wei.l...@citrix.com > --- > docs/man/xl.cfg.pod.5 |4 +++- > tools/libxl/libxl_create.c | 11 --- > 2 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 > index 8e4154f..fcca1cc 100644 > --- a/docs/man/xl.cfg.pod.5 > +++ b/docs/man/xl.cfg.pod.5 > @@ -437,7 +437,9 @@ UUID will be generated. > > =item B > > -Assign an XSM security label to this domain. > +Assign an XSM security label to this domain. By default a domain is > +assigned the label B, which is defined in > +the default policy. > > =item B > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > index f0da7dc..4dd2ec2 100644 > --- a/tools/libxl/libxl_create.c > +++ b/tools/libxl/libxl_create.c > @@ -42,6 +42,11 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc, > libxl_defbool_setdefault(&c_info->run_hotplug_scripts, true); > libxl_defbool_setdefault(&c_info->driver_domain, false); > > +if (!c_info->ssid_label) { > +c_info->ssid_label = libxl__strdup(NOGC, "system_u:system_r:domU_t"); > +LOG(INFO, "Using default ssid_label: %s", c_info->ssid_label); I don't think this is right. For one, the label you hardcoded here is defined in the policy we ship. It doesn't necessarily exist in the policy that is loaded by system admin. Another thing, as Julien said, is that this generates a warning in Xen that is not compiled with XSM support. By definition if you don't label a domain, it should be labeled as "unlabeled". We already do the right thing. Wei. > +} > + > return 0; > } > > @@ -802,6 +807,9 @@ static void initiate_domain_create(libxl__egc *egc, > > domid = 0; > > +ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info); > +if (ret) goto error_out; > + > if (d_config->c_info.ssid_label) { > char *s = d_config->c_info.ssid_label; > ret = libxl_flask_context_to_sid(ctx, s, strlen(s), > @@ -887,9 +895,6 @@ static void initiate_domain_create(libxl__egc *egc, > goto error_out; > } > > -ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info); > -if (ret) goto error_out; > - > ret = libxl__domain_make(gc, d_config, &domid, &state->config); > if (ret) { > LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot make domain: %d", ret); > -- > 1.7.10.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: assigned a default ssid_label (XSM label) to guests
On Thu, 2015-05-14 at 12:58 +0100, Wei Liu wrote: > On Thu, May 14, 2015 at 11:33:45AM +0100, Ian Campbell wrote: > > system_u:system_r:domU_t is defined in the default policy and makes as > > much sense as anything for a default. > > > > This change required moving the call to domain_create_info_setdefault > > to be before the ssid_label is translated into ssidref, which also > > moves it before some other stuff which consumes things from c_info, > > which is correct since setdefault should always be called first. Apart > > from the SSID handling there should be no functional change (since > > setdefault doesn't actually act on anything which that other stuff > > uses). > > > > There is no need to set exec_ssid_label since the default is to leave > > the domain using the ssid_label after build. > > > > I haven't done anything with the device model ssid. > > > > Signed-off-by: Ian Campbell > > Cc: Daniel De Graaf > > Cc: wei.l...@citrix.com > > --- > > docs/man/xl.cfg.pod.5 |4 +++- > > tools/libxl/libxl_create.c | 11 --- > > 2 files changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 > > index 8e4154f..fcca1cc 100644 > > --- a/docs/man/xl.cfg.pod.5 > > +++ b/docs/man/xl.cfg.pod.5 > > @@ -437,7 +437,9 @@ UUID will be generated. > > > > =item B > > > > -Assign an XSM security label to this domain. > > +Assign an XSM security label to this domain. By default a domain is > > +assigned the label B, which is defined in > > +the default policy. > > > > =item B > > > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > > index f0da7dc..4dd2ec2 100644 > > --- a/tools/libxl/libxl_create.c > > +++ b/tools/libxl/libxl_create.c > > @@ -42,6 +42,11 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc, > > libxl_defbool_setdefault(&c_info->run_hotplug_scripts, true); > > libxl_defbool_setdefault(&c_info->driver_domain, false); > > > > +if (!c_info->ssid_label) { > > +c_info->ssid_label = libxl__strdup(NOGC, > > "system_u:system_r:domU_t"); > > +LOG(INFO, "Using default ssid_label: %s", c_info->ssid_label); > > I don't think this is right. For one, the label you hardcoded here > is defined in the policy we ship. It doesn't necessarily exist in the > policy that is loaded by system admin. Personally I think that's fine, you either use the default, or you make sure your custom policy has a domU_t role (a very reasonable thing to have) or you specify something custom for every domain. > Another thing, as Julien said, is that this generates a warning in Xen > that is not compiled with XSM support. > > By definition if you don't label a domain, it should be labeled as > "unlabeled". We already do the right thing. So how come osstest is failing? What should we do instead? Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 00/16] Misc patches to aid migration v2 Remus support
On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote: > This is the combination of Andrew Cooper's misc patches and mine > to aid migration v2 Remus support. Applied, thanks. I made a few changes to some commit logs, I hope that's ok. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: assigned a default ssid_label (XSM label) to guests
On Thu, May 14, 2015 at 01:32:04PM +0100, Ian Campbell wrote: > On Thu, 2015-05-14 at 12:58 +0100, Wei Liu wrote: > > On Thu, May 14, 2015 at 11:33:45AM +0100, Ian Campbell wrote: > > > system_u:system_r:domU_t is defined in the default policy and makes as > > > much sense as anything for a default. > > > > > > This change required moving the call to domain_create_info_setdefault > > > to be before the ssid_label is translated into ssidref, which also > > > moves it before some other stuff which consumes things from c_info, > > > which is correct since setdefault should always be called first. Apart > > > from the SSID handling there should be no functional change (since > > > setdefault doesn't actually act on anything which that other stuff > > > uses). > > > > > > There is no need to set exec_ssid_label since the default is to leave > > > the domain using the ssid_label after build. > > > > > > I haven't done anything with the device model ssid. > > > > > > Signed-off-by: Ian Campbell > > > Cc: Daniel De Graaf > > > Cc: wei.l...@citrix.com > > > --- > > > docs/man/xl.cfg.pod.5 |4 +++- > > > tools/libxl/libxl_create.c | 11 --- > > > 2 files changed, 11 insertions(+), 4 deletions(-) > > > > > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 > > > index 8e4154f..fcca1cc 100644 > > > --- a/docs/man/xl.cfg.pod.5 > > > +++ b/docs/man/xl.cfg.pod.5 > > > @@ -437,7 +437,9 @@ UUID will be generated. > > > > > > =item B > > > > > > -Assign an XSM security label to this domain. > > > +Assign an XSM security label to this domain. By default a domain is > > > +assigned the label B, which is defined in > > > +the default policy. > > > > > > =item B > > > > > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > > > index f0da7dc..4dd2ec2 100644 > > > --- a/tools/libxl/libxl_create.c > > > +++ b/tools/libxl/libxl_create.c > > > @@ -42,6 +42,11 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc, > > > libxl_defbool_setdefault(&c_info->run_hotplug_scripts, true); > > > libxl_defbool_setdefault(&c_info->driver_domain, false); > > > > > > +if (!c_info->ssid_label) { > > > +c_info->ssid_label = libxl__strdup(NOGC, > > > "system_u:system_r:domU_t"); > > > +LOG(INFO, "Using default ssid_label: %s", c_info->ssid_label); > > > > I don't think this is right. For one, the label you hardcoded here > > is defined in the policy we ship. It doesn't necessarily exist in the > > policy that is loaded by system admin. > > Personally I think that's fine, you either use the default, or you make > sure your custom policy has a domU_t role (a very reasonable thing to > have) or you specify something custom for every domain. > > > Another thing, as Julien said, is that this generates a warning in Xen > > that is not compiled with XSM support. > > > > By definition if you don't label a domain, it should be labeled as > > "unlabeled". We already do the right thing. > > So how come osstest is failing? What should we do instead? > AIUI current policy doesn't allow unlabeled domain to do certain things. Maybe we can figure out a way to tune the policy to give certain permissions to unlabeled domain. This would need input from Daniel to know if it is achievable. Wei. > Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] xl: Support (by ignoring) xl migrate --live
xm migrate would do non-live migration (effectively, save, transfer and restore) by default, unless you specified --live. xl migrate always does live migration. Honour (by ignoring) --live for compatibility with old callers. Document this. (This patch should be backported as far as possible.) Reported-by: Matthew Vernon Signed-off-by: Ian Jackson CC: Matthew Vernon --- docs/man/xl.pod.1|8 tools/libxl/xl_cmdimpl.c |6 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1 index 16783c8..217a0ab 100644 --- a/docs/man/xl.pod.1 +++ b/docs/man/xl.pod.1 @@ -1513,6 +1513,14 @@ monitor types are: =back +=head1 IGNORED FOR COMPATIBILITY WITH XM + +xl is mostly command-line compatible with the old xm utility used with +the old Python xend. For compatibility, the following options are +ignored: + +=item B + =head1 TO BE DOCUMENTED We need better documentation for: diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 648ca08..50a929a 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -4498,6 +4498,7 @@ int main_migrate(int argc, char **argv) int opt, daemonize = 1, monitor = 1, debug = 0; static struct option opts[] = { {"debug", 0, 0, 0x100}, +{"live", 0, 0, 0x200}, COMMON_LONG_OPTS, {0, 0, 0, 0} }; @@ -4516,9 +4517,12 @@ int main_migrate(int argc, char **argv) daemonize = 0; monitor = 0; break; -case 0x100: +case 0x100: /* --debug */ debug = 1; break; +case 0x200: /* --live */ +/* ignored for compatibility with xm */ +break; } domid = find_domain(argv[optind]); -- 1.7.10.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
Paolo Bonzini writes: > On 14/05/2015 14:02, Markus Armbruster wrote: >> It should certainly be off for pc-q35-2.4 and newer. Real Q35 boards >> commonly don't have an FDC (depends on the Super I/O chip used). >> >> We may want to keep it off for pc-i440fx-2.4 and newer. I doubt >> there's a real i440FX without an FDC, but our virtual i440FX is quite >> unlike a real one in other ways already. > > That would break libvirt for people upgrading from 2.3 to 2.4. So it's > more like pc-i440fx-3.0 and pc-q35-3.0. What exactly breaks when? > Unless for q35 we decide to > break everything and retroactively nuke the controller. > > (I'm still not sure why we have backwards-compatible machine types for q35). Beats me :) [...] ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH Remus v5 1/2] libxc/save: implement Remus checkpointed save
On Thu, 2015-05-14 at 18:06 +0800, Yang Hongyang wrote: > With Remus, the save flow should be: > live migration->{ periodically save(checkpointed save) } > > Signed-off-by: Yang Hongyang > Reviewed-by: Andrew Cooper > CC: Ian Campbell > CC: Ian Jackson > CC: Wei Liu > --- > tools/libxc/xc_sr_save.c | 80 > > 1 file changed, 61 insertions(+), 19 deletions(-) > > diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c > index 1d0a46d..1c5d199 100644 > --- a/tools/libxc/xc_sr_save.c > +++ b/tools/libxc/xc_sr_save.c > @@ -57,6 +57,16 @@ static int write_end_record(struct xc_sr_context *ctx) > } > > /* > + * Writes an CHECKPOINT record into the stream. "a CHECKPOINT" > + */ > +static int write_checkpoint_record(struct xc_sr_context *ctx) > +{ > +struct xc_sr_record checkpoint = { REC_TYPE_CHECKPOINT, 0, NULL }; > + > +return write_record(ctx, &checkpoint); > +} > + > +/* > * Writes a batch of memory as a PAGE_DATA record into the stream. The batch > * is constructed in ctx->save.batch_pfns. > * > @@ -467,6 +477,14 @@ static int send_domain_memory_live(struct xc_sr_context > *ctx) > DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap, > &ctx->save.dirty_bitmap_hbuf); > > +/* > + * With Remus, we will enter checkpointed save after live migration. > + * In checkpointed save loop, we skip the live part and pause straight > + * away to send dirty pages between checkpoints. > + */ > +if ( !ctx->save.live ) > +goto last_iter; Rather than use goto would it work to refactor everything from here to the label into some sort of helper and just call that in the "actually live" case? Or perhaps everything from the label to the end should be a helper function which the caller can also use in thecheckpoint case instead of calling send_domain_memory_live (and which s_d_m_l also calls of course). > +if ( ctx->save.checkpointed ) > +{ > +if ( ctx->save.live ) > +{ > +/* End of live migration, we are sending checkpointed stream > */ > +ctx->save.live = false; I think I'm misunderstanding either the purpose of this code or the comment (or both). Is it the case that a checkpoint starts with an iteration of live (to transfer everything over) and then drops into sending periodical non-live updates at each checkpoint? If so then I think a more useful comment would be: /* * We have now completed the initial live portion of the checkpoint * process. Therefore switch into periodically sending synchronous * batches of pages. */ Personally I don't have a problem with just a direct assignment without the if, since assigning false to an already flase value is a nop. > +} > + > +rc = write_checkpoint_record(ctx); > +if ( rc ) > +goto err; > + > +ctx->save.callbacks->postcopy(ctx->save.callbacks->data); > + > +rc = ctx->save.callbacks->checkpoint(ctx->save.callbacks->data); > +if ( rc > 0 ) > +xc_report_progress_single(xch, "Checkpointed save"); > +else > +ctx->save.checkpointed = false; > +} > +} while ( ctx->save.checkpointed ); > > xc_report_progress_single(xch, "End of stream"); > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel