Re: [PATCH 1/7] storage: Remove unused attribute conn from 'checkPool' callback

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

 



On Wed, Mar 25, 2015 at 12:19:18 -0400, John Ferlan wrote:
> 
> 
> On 03/24/2015 06:06 AM, Erik Skultety wrote:
> > The checkPool callback should be used when updating states of all pools
> > during storageStateInitialize, not storageDriverAutostart, otherwise we
> > can't start a domain which mounts a volume after daemons restarted. This
> > is because qemuProcessReconnect is called earlier than
> > storageDriverAutostart which marks the pool as active, so that the
> > domain can mount a volume from this pool successfully.
> > In order to fix this, conn attribute has to be discarded from the
> > callback, because storageStateInitialize doesn't have a connection yet.
> > (it's not used anyway)
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733

This patch definitely does not resolve this^^.

> > ---
> >  src/storage/storage_backend.h         | 3 +--
> >  src/storage/storage_backend_fs.c      | 3 +--
> >  src/storage/storage_backend_iscsi.c   | 3 +--
> >  src/storage/storage_backend_logical.c | 3 +--
> >  src/storage/storage_backend_mpath.c   | 3 +--
> >  src/storage/storage_backend_scsi.c    | 3 +--
> >  src/storage/storage_backend_zfs.c     | 3 +--
> >  src/storage/storage_driver.c          | 2 +-
> >  8 files changed, 8 insertions(+), 15 deletions(-)
> > 
> 
> I'm not convinced we can remove 'conn'. Something about backend or
> backward compatibility. Since it's not used by any backend anyway,
> there's (more or less) no harm in keeping it.

The storage driver backend APIs are internal so it's okay to change
Additionally I recall that it will simplify some of the further patches.

ACK

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]