[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [v8][PATCH 04/17] update the existing hypercall to support XEN_DOMCTL_set_rdm



On 2014/12/9 16:19, Jan Beulich wrote:
On 09.12.14 at 08:47, <tiejun.chen@xxxxxxxxx> wrote:
On 2014/12/8 16:51, Jan Beulich wrote:
The whole "if-copy-unlock-and-return-EFAULT-otherwise-increment"
is identical and can be factored out pretty easily afaict.

What about this?

struct get_reserved_device_memory {
      struct xen_reserved_device_memory_map map;
      unsigned int used_entries;
      struct domain *domain;
};

static int get_reserved_device_memory(xen_pfn_t start, xen_ulong_t nr,
                                        u32 id, void *ctxt)
{
      struct get_reserved_device_memory *grdm = ctxt;
      struct domain *d = grdm->domain;
      unsigned int i, hit_one = 0;
      u32 sbdf;
      struct xen_reserved_device_memory rdm = {
          .start_pfn = start, .nr_pages = nr
      };

      if ( !d->arch.hvm_domain.pci_force )
      {
          for ( i = 0; i < d->arch.hvm_domain.num_pcidevs; i++ )
          {
              sbdf = PCI_SBDF2(d->arch.hvm_domain.pcidevs[i].seg,
                               d->arch.hvm_domain.pcidevs[i].bus,
                               d->arch.hvm_domain.pcidevs[i].devfn);
              if ( sbdf == id )
              {
                  hit_one = 1;
                  break;
              }
          }

          if ( !hit_one )
              return 0;
      }

Why do you always pick other than the simplest possible solution?

I don't intend it to be, but I may go a complicated way, even a wrong way, based on my understanding. But as one main maintainer, if you always say to me in such a reproachful word more than once, I have to consider you may hint constantly I'm not a suitable candidate to finish this. Its fair to me, I'd really like to quit this to ask my manager if it can deliver to other guy to make sure this can move forward.

You don't need a separate variable here, you can simply check
whether i reached d->arch.hvm_domain.num_pcidevs after the
loop. And even if you added a variable, it would want to be a

Are you saying this?

        if ( i == d->arch.hvm_domain.num_pcidevs )
                return 0;

But if the last one happens to one hit, 'i' is equal to d->arch.hvm_domain.num_pcidevs.

Thanks
Tiejun

bool_t one with the way you use it.

Jan




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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.