Re: [Xen-devel] [v3][PATCH 12/16] tools/libxl: passes rdm reservation policy

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Jun 15, 2015 at 09:26:30AM +0800, Chen, Tiejun wrote:
[...]
> >>  out:
> >>      if (!libxl_is_stubdom(ctx, domid, NULL)) {
> >>+        if (pcidev->rdm_reserve == LIBXL_RDM_RESERVE_FLAG_RELAXED) {
> >>+            flag = XEN_DOMCTL_DEV_RDM_RELAXED;
> >>+        } else if (pcidev->rdm_reserve == LIBXL_RDM_RESERVE_FLAG_STRICT) {
> >>+            flag = XEN_DOMCTL_DEV_RDM_STRICT;
> >>+        } else {
> >>+            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unkwon rdm check 
> >>flag.");
> >
> >Typo "unkwon" and use LOG(ERROR,...).
> 
> Will fix that typo, s/unkwon/unknown, but are you sure we should use LOG()
> here? Because this function always uses LIBXL__LOG_ERRNO(),
> 

Yeah. Consistency is also a strong argument. I won't force my opinion on
you. Fixing the typo is good enough for me.

> >
> >>+            return ERROR_FAIL;
> >>+        }
> >>          rc = xc_assign_device(ctx->xch, domid, pcidev_encode_bdf(pcidev), 
> >> flag);
> >>          if (rc < 0 && (hvm || errno != ENOSYS)) {
> >>              LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_assign_device 
> >> failed");
> 
[...]
> >>  }
> >>
> >>diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> >>index 7f4759b..552fbec 100644
> >>--- a/tools/libxl/xl_cmdtable.c
> >>+++ b/tools/libxl/xl_cmdtable.c
> >>@@ -88,7 +88,7 @@ struct cmd_spec cmd_table[] = {
> >>      { "pci-attach",
> >>        &main_pciattach, 0, 1,
> >>        "Insert a new pass-through pci device",
> >>-      "<Domain> <BDF> [Virtual Slot]",
> >>+      "<Domain> <BDF> [Virtual Slot] <policy to reserve 
> >>rdm['strice'|'relaxed']>",
> >
> >Should use "[]" to indicate it's optional.
> >
> 
> What about this?
> 
> "<Domain> <BDF> [Virtual Slot] [policy to reserve rdm<'strice'|'relaxed'>]",
> 

Fine by me.

Wei.

> Thanks
> Tiejun

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