On 4/5/23 19:56, Shiva wrote: > Greetings Sir > > My apologies for the incorrect formats as I am a beginner to git. Thank > you very much for your corrections. That's okay and GSoC was invented specifically for this reason. To attract students into Open Source and its development. But at the same time, review bandwidth is precious and therefore it's important to listen to feedback. I'm glad to see v2 worked out better. > > I have a few questions: > > Can Questions and doubts be posted within the same mail that consists a > patch file? Or should they be put in a separate email (as I am > attempting to do now)? It's okay to send it as two separate emails even. I mean, usually, when we work on something and for instance can't make a decision (e.g. on a design or something), we often just write an e-mail to the list, explaining what it is we're working on and that we face this decision, what are the options, etc. And other developers reply with their thoughts. > > As per your instruction to make the functions static, I was able to make > the below function (created by me) static by calling it from > virDomainGraphicsDefListensValidate() within domain_validate.c > > static int > virDomainGraphicsListenDefValidate(const virDomainGraphicsListenDef *def, > const virDomainGraphicsDef *graphics) Yep, that's correct. > > Similarly, > In this patch: Link > <https://listman.redhat.com/archives/libvir-list/2023-April/239205.html> > I had created a function named; > > int virDomainStorageNetHostSocketValidate(const virStorageNetHostDef *host, > const char *transport); > > that removes the validation checks of socket in > virDomainStorageNetworkParseHost(). > While I found a function to call Graphics Validation within validate.c, > I am unable to find a function that can be used to call > NetworkHostSocketValidate() from within domain_validate.c. Could you > please provide a pointer on how it could be done? Yeah, you may need to write new code. As I've said, we're only gradually moving code into domain_validate.c. But let me see: So virDomainStorageNetworkParseHost() is called from exactly two places (identified by 'git grep -npC10 virDomainStorageNetworkParseHost'): virDomainBackupDefParseXML virDomainStorageNetworkParseHosts Let's focus on the latter for now, we'll get back to the former soon. virDomainStorageNetworkParseHosts() is then called from the following places: virDomainHostdevSubsysSCSIiSCSIDefParseXML virDomainDiskSourceNetworkParse after repeating this process more times, we can find what device type might end up calling this function and what conditions lead to it. For instance, in this case it's a hostdev that has scsi subdev, iscsi protocol, etc. Now, looking at domain_validate.c there's virDomainDeviceDefValidateInternal() which calls virDomainHostdevDefValidate() for hostdevs, and then has a block of checks for scsi subsys, but no further code for iscsi protocol. So that looks like a place to add the check. Basically, we're trying to reconstruct the path from domain_conf.c that lead to the check in domain_validate.c. And now let's return back to the other caller of the function: virDomainBackupDefParseXML(). Unfortunately, the XML that's parsed there is not validated - there's no vir*Validate() function called; so if this check is removed and moved into domain_validate.c then this path would be left buggy. After all, it's probably okay to leave the check there. Introducing validation to backup XMLs is solvable, but definitely not a bite sized task for newcomer. Michal
