[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 09.12.14 at 10:12, <tiejun.chen@xxxxxxxxx> wrote: > 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; Yes. Or use >=. > But if the last one happens to one hit, 'i' is equal to > d->arch.hvm_domain.num_pcidevs. No, when the last one hits, i == d->arch.hvm_domain.num_pcidevs - 1. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |