Catching exception when running stat on non-existent volume

[arielsalvo]

No description provided.

[plribeiro3000]

Could you define an exception instead of rescue everything?
Otherwise this code will just swallow any other issues in that code.

[plribeiro3000]

Perhaps Libvirt::RetrieveError ?

[arielsalvo]

This is related to issue #13 .
I don't know that much Ruby to pinpoint the exception that's being raised.
If you can name the exception, I can change the code easily to comply.

[plribeiro3000]

I would say to define it like:

def volume_to_attributes(vol)
  format_type = xml_element(vol.xml_desc, "/volume/target/format", "type") rescue nil # not all volumes have types, e.g. LVM
  return nil if format_type == "dir"

  {
    :pool_name   => vol.pool.name,
    :key         => vol.key,
    :id          => vol.key,
    :path        => vol.path,
    :name        => vol.name,
    :format_type => format_type,
    :allocation  => bytes_to_gb(vol.info.allocation),
    :capacity    => bytes_to_gb(vol.info.capacity),
  }
rescue Libvirt::RetrieveError
  nil
end

[arielsalvo]

How about this new version?
Or do you prefer I omit the "begin-end" block?

[plribeiro3000]

Seems good to me. Can you confirm it is working with this change?

[arielsalvo]

I tested it by allowing jenkins to run multiple configurations and multiple projects in parallel.
It seems to have done the trick regarding volumes. Basically, this change ignores volumes that are not ready yet, so it should be OK.

I got a similar error from the list_domains.rb file (I added the comment in the issue) but it was rare. I think vagrant/libvirt takes a lot more time time bringing up a volume than a domain, so it makes a lot of sense that list_volumes should fail a lot more often than list_domains.

[plribeiro3000]

👍

[plribeiro3000]

Thank you for your time working on this!

[arielsalvo]

Remember to bump the version.... I don't know how you handle that in this project

[plribeiro3000]

@arielsalvo Yeah. I'm actually part of the fog core team and help review stuff but regarding fog-libvirt i don't have permission on rubygem so i can't do a release.

@strzibny is your man here. =)

[strzibny]

Yeah, I can do a release with this included. Edit: And we should inctroduce CHANGELOG.

[plribeiro3000]

Awesome. Thx. Let US know when The realeas is out please. Thanks!

Em sex, 19 de fev de 2016 03:54, Josef Strzibny notifications@github.com
escreveu:

Yeah, I can do a release with this included.

—
Reply to this email directly or view it on GitHub
#14 (comment).