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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Jun 03, 2015 at 10:58:31AM +0800, Chen, Tiejun wrote:
> On 2015/6/3 0:36, Wei Liu wrote:
> >On Fri, May 22, 2015 at 05:35:08PM +0800, Tiejun Chen wrote:
> >>This patch passes rdm reservation policy to xc_assign_device() so the policy
> >>is checked when assigning devices to a VM.
> >>
> >>Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
> >>---
> >>  tools/libxc/include/xenctrl.h       |  3 ++-
> >>  tools/libxc/xc_domain.c             |  4 +++-
> >>  tools/libxl/libxl_pci.c             | 11 ++++++++++-
> >>  tools/libxl/xl_cmdimpl.c            | 23 +++++++++++++++++++----
> >>  tools/libxl/xl_cmdtable.c           |  2 +-
> >
> >Where is document for the new options you added to xl pci commands?
> 
> Looks I'm missing to describe something specific to pci-attach?
> 
> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> index 4eb929d..2ebfd54 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -1368,10 +1368,15 @@ it will also attempt to re-bind the device to its
> original driver, making it
>  usable by Domain 0 again.  If the device is not bound to pciback, it will
>  return success.
> 
> -=item B<pci-attach> I<domain-id> I<BDF>
> +=item B<pci-attach> I<domain-id> I<BDF> I<rdm policy>
> 

The way you put it here suggests that "rdm policy" is mandatory. I don't
think this is the case?

If it is not mandatory, write [I<rdm>].

>  Hot-plug a new pass-through pci device to the specified domain.
>  B<BDF> is the PCI Bus/Device/Function of the physical device to
> pass-through.
> +B<rdm policy> is about how to handle conflict between reserving reserved
> device
> +memory and guest address space. "strict" means an unsolved conflict leads
> to
> +immediate VM crash, while "relaxed" allows VM moving forward with a warning
> +message thrown out. Here "strict" is default.
> +
> 
>  =item B<pci-detach> [I<-f>] I<domain-id> I<BDF>
> 
> 
> >
> >BTW you might want to consider rearrange patches in this series so that
> 
> Yes, this is really what I intend to do.
> 
> >you keep the tree bisectable.
> 
> Overall, I can separate this series as several parts,
> 
> #1. Introduce our policy configuration on tools side
> #2. Interact with Hypervisor to get rdm info
> #3. Implement our policy with rdm info on tool side
> #4. Make hvmloader to align our policy
> 
> If you already see something obviously wrong, let me know.
> 

I think all toolstack patches should come after hypervisor and hvmloader
patches. And then within toolstack patches, libxc patches should come
before libxl patches, libxl patches should come before xl patches.

The pattern is clear. Patches that are late in the series make use of
functionalities provided by early patches. Breaking this pattern is
definitely going to break bisection.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel