Re: [libvirt] [PATCH 1/2] storage: Fix autostart of pool with "fc_host" type adapter

On 01/07/2014 06:07 AM, Osier Yang wrote: > On 07/01/14 01:21, John Ferlan wrote: >> On 01/06/2014 05:19 AM, Osier Yang wrote: >>> The "checkPool" is a bit different for pool with "fc_host" >>> type source adapter, since the vHBA it's based on might be >>> not created yet (it's created by "startPool", which is >>> involked after "checkPool" in storageDriverAutostart). So it >>> should not fail, otherwise the "autostart" of the pool will >>> fail either. >>> >>> The problem is easy to reproduce: >>> * Enable "autostart" for the pool >>> * Restart libvirtd service >>> * Check the pool's state >>> --- >>> src/storage/storage_backend_scsi.c | 14 ++++++++++++-- >>> 1 file changed, 12 insertions(+), 2 deletions(-) >>> >> Not sure this is the right thing to do. With this change it doesn't >> matter what getAdapterName() returns for fc_host's... > It matters with if "*isActive" is true or false in the end. We don't > need to try to start the pool if it's already active. > Fair enough; however, let's consider the failure case. On failure, a message is reported and name == NULL. Do we need to clear that error now? I think my original thoughts were along the lines of having getAdapterName do more of the heavy lifting. Have both check and start call it. It would call the createVport which would be mostly unchanged, except for the virFileWaitForDevices() which could move to start...