Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes 3177: a 'G' suffix is automatically appended to libvirt volume capacity value if none was specified #920

Closed
wants to merge 1 commit into from

Conversation

dmitri-d
Copy link
Member

@dmitri-d dmitri-d commented Oct 1, 2013

I think the UI to volume capacity could be improved. libvirt internally stores volume sizes in bytes, but allows to specify the size in a variety of units. We could switch to follow the same convention.

It's a bit confusing atm, as libvirt expects a suffix to be present, but we only support one -- "G".

end

def validate_volume_capacity(vol)
if vol.capacity.to_s.empty? or /^\d+G?$/.match(vol.capacity.to_s).nil?
raise _("Please specify volume size. You may optionally use suffix 'G' to specify volume size in Gigabytes.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Foreman::Exception should be used here I think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A RuntimeError is used in a couple of other spots here -- should those be changed too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd be good, thanks

@domcleal
Copy link
Contributor

domcleal commented Oct 1, 2013

Could you file a redmine ticket about this issue please? We don't use BZs.

@dmitri-d
Copy link
Member Author

dmitri-d commented Oct 1, 2013

@domcleal: ping?

@@ -107,7 +107,7 @@ def create_vm args = { }

def console uuid
vm = find_vm_by_uuid(uuid)
raise "VM is not running!" unless vm.ready?
raise Foreman::Exception.new(_("VM is not running!")) unless vm.ready?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These need to be N_() when using Foreman::Exception, as it takes the English string to compute the message, then runs it through _() when rendered.

@dmitri-d
Copy link
Member Author

dmitri-d commented Oct 2, 2013

@domcleal: ping?

@domcleal
Copy link
Contributor

domcleal commented Oct 2, 2013

Thanks @witlessbird! Merged as dab82c9 for Foreman 1.4.0.

@domcleal domcleal closed this Oct 2, 2013
@dmitri-d dmitri-d deleted the 974253 branch December 13, 2017 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants