Re: [PATCH] test_driver: add testUpdateDeviceFlags implementation
On Wed, Jul 03, 2024 at 09:44:47AM +0200, Michal Prívozník wrote:
>> LIVE is not supported and thus shouldn't be in list of
supported flags.
>
> I was a bit unclear on the test hypervisor case - in a sense everything is both
> LIVE *and* CONFIG :)
I see what you mean, but no. The fact that libvirt doesn't really store
XMLs on disk for inactive domains doesn't necessarily mean there's no
distinction. The aim of test driver is to mimic real life scenarios as
closely as possible, so that when somebody is developing an app on top
of libvirt they can use the test driver instead of spawning real VMs and
thus test their app quickly. Therefore, the distinction between inactive
and live XMLs must be preserved.
I get the rationale, but now we're inconsistent with our previous contribution:
10247 static int
10248 testDomainAttachDeviceLiveAndConfig(virDomainObj *vm,
10249 testDriver *driver,
10250 const char *xml,
10251 unsigned int flags)
10252 {
10253 g_autoptr(virDomainDeviceDef) devLive = NULL;
10254 unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
10255 VIR_DOMAIN_DEF_PARSE_ABI_UPDATE;
10256
10257 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE, -1);
Given that the test driver does not and cannot have a persisted configuration
(there is only the in-process ->vmdef), I'm confused why you made the new code
be:
10399 virCheckFlags(VIR_DOMAIN_AFFECT_CONFIG, -1);
in testDomainUpdateDeviceFlags().
For our application we actually do want to pass the LIVE flag - we never want to
modify any persistent configuration. We'd prefer not to have to carry a patch on
top for this.
regards
john