On Thu, Mar 24, 2022 at 21:39:56 +0530, Jamm02 wrote: > From: Moteen Shah <moteenshah.02@xxxxxxxxx> > > domain_conf.c: all the option collision total and... error messages in > virDomainDiskDefIotuneParse shifted to new function virDomainDefPostParseCheck I don't quite understand what you mean by this commit message. Is it meant to signal that it's just pure code movement? Additionally we put the impacted file in the summary line so eg. conf: Move validation from virDomainDiskDefIotuneParse into the validation callback Also per https://libvirt.org/hacking.html#developer-certificate-of-origin Your submission is missing a certification of compliance with the "Developer certificate of origin". > --- > src/conf/domain_conf.c | 86 ++++++++++++++++++++++-------------------- > 1 file changed, 45 insertions(+), 41 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 153954a0b0..f2480f37f6 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -6229,6 +6229,50 @@ virDomainDefPostParseCheckFailure(virDomainDef *def, > return 0; > } > > +static int > +virDomainDefPostParseCheck(virDomainDiskDef *def) This function is really miss-named since it operates on a "virDomainDiskDef" object it should contain that in the name as "virDomainDef" is a separate object Also I suppose you were told to move this to "post parse checks" but what was meant is the already existing set of functions in this case in src/conf/domain_validate.c since this code is just validation of the input. Just extracting this here makes not that much sense by itself. > +{ > + if ((def->blkdeviotune.total_bytes_sec && > + def->blkdeviotune.read_bytes_sec) || > + (def->blkdeviotune.total_bytes_sec && > + def->blkdeviotune.write_bytes_sec)) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("total and read/write bytes_sec " > + "cannot be set at the same time")); > + return -1; > + }
