Re: [PATCH] test_driver: add testUpdateDeviceFlags implementation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux