Re: [libvirt] [PATCHv3 10/12] qemu: conf: Add support for memory device cold(un)plug

On 03/18/2015 01:51 AM, Zhu Guihua wrote: > On 03/17/2015 10:19 PM, Peter Krempa wrote: >> Add a few helpers that allow to operate with memory device definitions >> on the domain config and use them to implement memory device coldplug in >> the qemu driver. >> --- >> >> Notes: >> Version 2: >> - no changes >> >> src/conf/domain_conf.c | 100 >> +++++++++++++++++++++++++++++++++++++++++++++++ >> src/conf/domain_conf.h | 10 +++++ >> src/libvirt_private.syms | 4 ++ >> src/qemu/qemu_driver.c | 15 ++++++- >> 4 files changed, 127 insertions(+), 2 deletions(-) >> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 8c2234f..1a02e46 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -12811,6 +12811,106 @@ virDomainRNGRemove(virDomainDefPtr def, >> } >> >> >> +static int >> +virDomainMemoryFindByDefInternal(virDomainDefPtr def, >> + virDomainMemoryDefPtr mem, >> + bool allowAddressFallback) >> +{ >> + size_t i; >> + >> + for (i = 0; i < def->nmems; i++) { >> + virDomainMemoryDefPtr tmp = def->mems[i]; >> + >> + /* address, if present */ >> + if (allowAddressFallback) { >> + if (tmp->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) >> + continue; >> + } else { >> + if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && >> + !virDomainDeviceInfoAddressIsEqual(&tmp->info, >> &mem->info)) >> + continue; >> + } >> + >> + /* alias, if present */ >> + if (mem->info.alias && >> + STRNEQ_NULLABLE(tmp->info.alias, mem->info.alias)) >> + continue; >> + >> + /* target info -> always present */ >> + if (tmp->model != mem->model || >> + tmp->targetNode != mem->targetNode || >> + tmp->size != mem->size) > I have tested your series with our qemu memory hot remove patch series, > here would be a possible error. > > When hotplug a memory device, its size has been aligned. So the compare for > size here would fail possiblely. > hmm.. Not sure that's necessary - although Peter can make the final determination... Commit id '57b215a' doesn't modify each def->mems[i] entry in qemuDomainAlignMemorySizes, rather it gets a value from virDomainDefSetMemoryInitial and then does the rounding. If the stored def->mems[i]->size value is/was modified, then I'd agree, but it doesn't appear to be that way. If there is a rounding of the value - then please just point it out

John > Thanks, > Zhu > >> + continue; >> + >> + /* source stuff -> match with device */ >> + if (tmp->pagesize != mem->pagesize) >> + continue; >> + >> + if (!virBitmapEqual(tmp->sourceNodes, mem->sourceNodes)) >> + continue; >> + >> + break; >> + } >> + >> + if (i == def->nmems) >> + return -1; >> + >> + return i; >> +} >> + >> + >> +int >> +virDomainMemoryFindByDef(virDomainDefPtr def, >> + virDomainMemoryDefPtr mem) >> +{ >> + return virDomainMemoryFindByDefInternal(def, mem, false); >> +} >> + > [...] > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list .