Re: [PATCH 8/8] Revert "conf: Validate disk lun using correct types"

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

 



On Fri, May 06, 2016 at 11:19:05 -0400, John Ferlan wrote:
> 
> 
> On 05/02/2016 10:32 AM, Peter Krempa wrote:
> > This reverts commit c79ebf53b5fe0a33bf407b3bcb49e3a27ec97eb4.
> > 
> > We can't just add checks to the XML parser once we've accepted such
> > configuration in the past.
> > ---
> >  src/conf/domain_conf.c   | 22 ----------------------
> >  tests/qemuxml2argvtest.c |  3 +--
> >  2 files changed, 1 insertion(+), 24 deletions(-)
> > 
> 
> There was a bz associated with that commit - that'll need to be
> addressed in some manner...

Well, the initial assesment of that BZ was wrong. This should have been
fixed in virt manager at that point.

> While I understand your point here, the configuration didn't work - that
> is it couldn't be started anyway so there could not be a domain running
> with that configuration and thus it wouldn't disappear on a subsequent
> reload, hence why checking the config and rejecting "earlier" seemed

It does not kill any running domain, that's right. Defined domains still
vanish after that commit if they were defined before. That is still
unwanted.

> proper even though we hadn't rejected such a config when the
> "mode='host'" was first implemented.

Only when introducing a feature you are allowed to do a check that
rejects parsing XML, afterwards, no such thins should be added.

> 
> Commit 'c79ebf53b' is a followup of sorts to commit '33188c9f'...

'33188c9f' is okay. Startup time checks can be added.

> 
> For me it's a NACK, but someone else may feel differently

I still insist on reverting the code. This should be fixed in virt
manager as in libvirt it creates a vanishing VM problem.

Peter

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[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]