On Mon, Nov 22, 2021 at 10:50:18AM +0000, Oleksandr Andrushchenko wrote:
>
>
> On 22.11.21 12:43, Jan Beulich wrote:
> > On 22.11.2021 11:27, Roger Pau Monné wrote:
> >> On Mon, Nov 22, 2021 at 11:28:25AM +0200, Oleksandr Andrushchenko wrote:
> >>> --- a/xen/drivers/vpci/header.c
> >>> +++ b/xen/drivers/vpci/header.c
> >>> @@ -206,12 +206,16 @@ static void defer_map(struct domain *d, struct
> >>> pci_dev *pdev,
> >>> static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool
> >>> rom_only)
> >>> {
> >>> struct vpci_header *header = &pdev->vpci->header;
> >>> - struct rangeset *mem = rangeset_new(NULL, NULL, 0);
> >>> + struct rangeset *mem;
> >>> + char str[32];
> >>> struct pci_dev *tmp, *dev = NULL;
> >>> const struct vpci_msix *msix = pdev->vpci->msix;
> >>> unsigned int i;
> >>> int rc;
> >>>
> >>> + snprintf(str, sizeof(str), "%pp", &pdev->sbdf);
> >>> + mem = rangeset_new(NULL, str, RANGESETF_no_print);
> >> You are still not adding the rangeset to the domain list, as the first
> >> parameter passed here in NULL instead of a domain struct.
> >>
> >> Given the current short living of the rangesets I'm not sure it makes
> >> much sense to link them to the domain ATM, but I guess this is kind of
> >> a preparatory change as other patches you have will have the
> >> rangesets permanent as long as the device is assigned to a domain.
> >>
> >> Likely the above reasoning (or the appropriate one) should be added to
> >> the commit message.
> If I fold then there is no reason to add the comment, right?
I find detailed log messages never hurt, so in the patch where you
squash the chunk below I would add that as part of making the
rangesets permanent they are also linked to the domain struct in order
to properly track them.
Thanks, Roger.