Re: [Xen-devel] [RFC][PATCH 08/13] tools: extend xc_assign_device() to support rdm reservation policy

[Chen, Tiejun]

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

[Andrew Cooper]

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

[Andrew Cooper]

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

[osstest service user]

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

[Chen, Tiejun]

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

[osstest service user]

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

[Razvan Cojocaru]

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

[Ian Campbell]

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

[Ian Campbell]

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

[Yang Hongyang]

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

[Yang Hongyang]

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

[Yang Hongyang]

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

[Yang Hongyang]

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

[Yang Hongyang]

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.

[Yang Hongyang]

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

[Yang Hongyang]

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

[Yang Hongyang]

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()

[Yang Hongyang]

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

[Yang Hongyang]

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

[Yang Hongyang]

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

[Yang Hongyang]

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()

[Yang Hongyang]

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

[Yang Hongyang]

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

[Yang Hongyang]

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

[Yang Hongyang]

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

[Yang Hongyang]

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

[Yang Hongyang]

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

[Yang Hongyang]

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

[Yang Hongyang]

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

[Ian Campbell]

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

[Tim Deegan]

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

[George Dunlap]

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

[Andrew Cooper]

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

[Andrew Cooper]

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

[Dario Faggioli]

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

[Andrew Cooper]

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

[Ian Campbell]

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

[Tim Deegan]

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

[Tim Deegan]

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

[Tim Deegan]

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

[Ian Campbell]

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

[Yang Hongyang]

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

[Yang Hongyang]

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

[osstest service user]

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

[Yang Hongyang]

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

[Andrew Cooper]

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

[Julien Grall]

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

[Ian Campbell]

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

[Ian Campbell]

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

[Tim Deegan]

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 Jackson]

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 Jackson]

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

[Ian Campbell]

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

[Fabio Fantoni]

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

[Roger Pau Monné]

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

[Ian Campbell]

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

[Anthony PERARD]

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

[Fabio Fantoni]

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

[Stefano Stabellini]

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.

[Ian Campbell]

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

[Ian Campbell]

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

[Daniel P. Berrange]

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

[Ian Campbell]

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

[David Vrabel]

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

[David Vrabel]

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

[David Vrabel]

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

[David Vrabel]

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.

[Ian Campbell]

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

[Ian Campbell]

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

[Julien Grall]

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

[Tim Deegan]

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

[Ian Campbell]

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

[Ian Campbell]

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

[Ian Campbell]

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

[Ian Campbell]

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

[Ian Campbell]

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

[Ian Campbell]

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()

[Ian Campbell]

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

[Tim Deegan]

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

[Tim Deegan]

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

[Michael S. Tsirkin]

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()

[Ian Campbell]

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

[Michael S. Tsirkin]

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

[Ian Campbell]

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

[Ian Campbell]

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

[Ian Campbell]

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

[Ian Campbell]

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

[Paolo Bonzini]

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

[Michael S. Tsirkin]

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

[Markus Armbruster]

"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

[David Vrabel]

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

[Paolo Bonzini]

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.

[Ian Campbell]

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

[Wei Liu]

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

[Ian Campbell]

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

[Ian Campbell]

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

[Wei Liu]

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

[Ian Jackson]

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

[Markus Armbruster]

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

[Ian Campbell]

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