GNOME Bugzilla – Bug 680825
Disable "continue" until password (and such) are validated
Last modified: 2016-03-31 13:59:12 UTC
When setting up a Fedora VM in the wizard, the password is required, but the Continue button isn't disabled until it is provided. The user shouldn't be allowed to continue if all informations needed are not provided.
As discussed during with the designers, in this specific case, the root password can be randomly generated, the user we create is in the sudo group, so it's possible to become root this way. The fedora installer should be fine (?) with passwordless users.
(In reply to comment #1) > As discussed during with the designers, in this specific case, the root > password can be randomly generated, the user we create is in the sudo group, so > it's possible to become root this way. The fedora installer should be fine (?) > with passwordless users. I thought more about that later and I now recall the problem with passwordless user: You need to unset password from postinstallation script in the kickstart file but unfortunately anaconda runs postinstall before setting user account.
(In reply to comment #2) > (In reply to comment #1) > > As discussed during with the designers, in this specific case, the root > > password can be randomly generated, the user we create is in the sudo group, so > > it's possible to become root this way. The fedora installer should be fine (?) > > with passwordless users. > > I thought more about that later and I now recall the problem with passwordless > user: You need to unset password from postinstallation script in the kickstart > file but unfortunately anaconda runs postinstall before setting user account. Perhaps we can set random password for user as well for this case? autologin is setup for user anyways.
This would mean the user cannot do any administrative task on their box as they won't know the sudo password
Created attachment 220565 [details] [review] wizard: Keep user in 'setup' until all data is provided We used to fail with an appropriate error message when user tried to hit 'continue' in setup page without providing needed data. Recently we had a regression in that regard so user was only getting a "VM setup failed" error. This patch fixes the regression but with an alternative solution: Keep the 'continue' button disabled as long as user hasn't provided all the needed data.
Created attachment 220566 [details] [review] winxp,express: Require user to input product key
(In reply to comment #5) > Created an attachment (id=220565) [details] [review] > wizard: Keep user in 'setup' until all data is provided > > We used to fail with an appropriate error message when user tried to hit > 'continue' in setup page without providing needed data. Recently we had a > regression in that regard so user was only getting a "VM setup failed" > error. > > This patch fixes the regression but with an alternative solution: Keep the > 'continue' button disabled as long as user hasn't provided all the needed > data. How does the user know which data is required, why the button is disabled, ... ?
(In reply to comment #6) > Created an attachment (id=220566) [details] [review] > winxp,express: Require user to input product key It's not strictly required, the Windows installer will ask for the product key if the user did not enter it.
(In reply to comment #7) > (In reply to comment #5) > > Created an attachment (id=220565) [details] [review] [details] [review] > > wizard: Keep user in 'setup' until all data is provided > > > > We used to fail with an appropriate error message when user tried to hit > > 'continue' in setup page without providing needed data. Recently we had a > > regression in that regard so user was only getting a "VM setup failed" > > error. > > > > This patch fixes the regression but with an alternative solution: Keep the > > 'continue' button disabled as long as user hasn't provided all the needed > > data. > > How does the user know which data is required, why the button is disabled, ... > ? Good point. Currently the only indication is 'continue' button enabled/disabled. How about we either show a "*" prefix in the label of required fields or "Requied" label next to required fields? (In reply to comment #8) > (In reply to comment #6) > > Created an attachment (id=220566) [details] [review] [details] [review] > > winxp,express: Require user to input product key > > It's not strictly required, the Windows installer will ask for the product key > if the user did not enter it. Yeah but any input needed during installation breaks the concept of 'unattended installation' so one can argue that product key is required for unattended installation.
(In reply to comment #9) > (In reply to comment #7) > > (In reply to comment #5) > > > Created an attachment (id=220565) [details] [review] [details] [review] [details] [review] > > > wizard: Keep user in 'setup' until all data is provided > > > > > > We used to fail with an appropriate error message when user tried to hit > > > 'continue' in setup page without providing needed data. Recently we had a > > > regression in that regard so user was only getting a "VM setup failed" > > > error. > > > > > > This patch fixes the regression but with an alternative solution: Keep the > > > 'continue' button disabled as long as user hasn't provided all the needed > > > data. > > > > How does the user know which data is required, why the button is disabled, ... > > ? > > Good point. Currently the only indication is 'continue' button > enabled/disabled. How about we either show a "*" prefix in the label of > required fields or "Requied" label next to required fields? Imo the "password required" thing we used to have is good enough, and improvements we make to it should go through the design team first (since it seems the solution that was discussed at GUADEC can't work out). > Yeah but any input needed during installation breaks the concept of 'unattended > installation' so one can argue that product key is required for unattended > installation. Yep, but I don't know if it's worth an error/blocking forward progress.
(In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #7) > > > (In reply to comment #5) > > > > Created an attachment (id=220565) [details] [review] [details] [review] [details] [review] [details] [review] > > > > wizard: Keep user in 'setup' until all data is provided > > > > > > > > We used to fail with an appropriate error message when user tried to hit > > > > 'continue' in setup page without providing needed data. Recently we had a > > > > regression in that regard so user was only getting a "VM setup failed" > > > > error. > > > > > > > > This patch fixes the regression but with an alternative solution: Keep the > > > > 'continue' button disabled as long as user hasn't provided all the needed > > > > data. > > > > > > How does the user know which data is required, why the button is disabled, ... > > > ? > > > > Good point. Currently the only indication is 'continue' button > > enabled/disabled. How about we either show a "*" prefix in the label of > > required fields or "Requied" label next to required fields? > > Imo the "password required" thing we used to have is good enough, and > improvements we make to it should go through the design team first (since it > seems the solution that was discussed at GUADEC can't work out). This patch *is* implementing what designers said about required data in general. We can't have those error messages on hitting 'continue' if 'continue' button is not enabled while required data is not yet provided. About the password being required for fedora, I already mentioned the problem with that and we can address that issue separately as in that regard I'm not really proposing anything different than what we were already doing before the regression. > > Yeah but any input needed during installation breaks the concept of 'unattended > > installation' so one can argue that product key is required for unattended > > installation. > > Yep, but I don't know if it's worth an error/blocking forward progress. I think it is. Otherwise, UX will suck. We'll be giving the wrong impression to user that express install can be successfully launched. She launches it and goes afk and after an hour she comes back to see installation UI staring at her, asking for key to contiue.
(In reply to comment #11) > > This patch *is* implementing what designers said about required data in > general. It seems we forgot to ask how we should make it obvious that some data is required then. I only remember the fedora password discussion to be honest. > About the password being required for fedora, I already mentioned the problem > with that and we can address that issue separately as in that regard I'm not > really proposing anything different than what we were already doing before the > regression. Is it recorded in bugzilla, or was it just on IRC? I forgot.
(In reply to comment #12) > (In reply to comment #11) > > > > > This patch *is* implementing what designers said about required data in > > general. > > It seems we forgot to ask how we should make it obvious that some data is > required then. I only remember the fedora password discussion to be honest. > > > About the password being required for fedora, I already mentioned the problem > > with that and we can address that issue separately as in that regard I'm not > > really proposing anything different than what we were already doing before the > > regression. > > Is it recorded in bugzilla, or was it just on IRC? I forgot. It was on this bug but I had another chat with mccann on this: <zeenix> jimmac, mccann: https://bugzilla.gnome.org/show_bug.cgi?id=680825#c12 <bebot> Bug 680825: normal, Normal, ---, gnome-boxes-maint, UNCONFIRMED, Disable "continue" until password (and such) are validated <zeenix> how do we indicate which data on 'setup' page is required? <zeenix> "How about we either show a "*" prefix in the label of <zeenix> required fields or "Requied" label next to required fields?" <jimmac> what optional fields do we have? <zeenix> password is optional except for fedora <zeenix> if you recall, we had a dicussion about that <zeenix> but https://bugzilla.gnome.org/show_bug.cgi?id=680825#c2 <bebot> Bug 680825: normal, Normal, ---, gnome-boxes-maint, UNCONFIRMED, Disable "continue" until password (and such) are validated * cosimoc is now known as cosimoc_afk <mccann> didn't we decide that we didn't need a password? <zeenix> we did <zeenix> https://bugzilla.gnome.org/show_bug.cgi?id=680825#c2 <bebot> Bug 680825: normal, Normal, ---, gnome-boxes-maint, UNCONFIRMED, Disable "continue" until password (and such) are validated <zeenix> one thing i didn't try though was to unset passwd from gdm script (or some other script executed at first login..) <zeenix> s/gdm script/gdm config/ <mccann> so we don't use kickstart for these installs? <zeenix> we do <zeenix> kickstart doesn't have any way to say 'no password\ <zeenix> anyway, the question was how do we indicate required fields? <mccann> and the question is why are we asking for non required things? <zeenix> i'll give 'not requiring passwd' another try <zeenix> to allow user to set password if he/she wants to? <zeenix> the field comes from your mockups so you decide :) <zeenix> s/field/entry/ <mccann> if we don't require a password then we shouldn't be asking for one <zeenix> so user shouldn't be able to set password at this point? <mccann> these boxes are all owned only by the user by default <mccann> I don't see how that is needed at this point <zeenix> the password is also used for spice connection btw <zeenix> and if provided, user needs to provide it when connecting <zeenix> and user can connect from remote machine too (haven't tested this one though but should be possible) <mccann> I should be able to add a password later right? <zeenix> yes <mccann> how does remote access work? <zeenix> SPICE <mccann> not sure I understand how boxes should be providing remote access to personal vms <zeenix> you connect using virt-viewer, boxes etc <mccann> only when boxes is running? <zeenix> yes <mccann> that's WEIRD <zeenix> only to display <mccann> you should probably be using a local socket <mccann> right jrb? <zeenix> yeah, for local ones we do <zeenix> i-e local connections/clients <zeenix> elmarco: would know better <mccann> if I create a box I don't want it showing up on the network <zeenix> dont think its advertized <mccann> i don't think it should be accessible <mccann> certainly not by default <zeenix> well, i have no opinions on that <mccann> if that is a feature we want then there should be an option in the properties and that can suggest adding a password at that time <zeenix> sounds good to me <elmarco> mccann: a different password? for each VM? * jbicha (~jeremy@64.28.198.199) has joined #boxes <mccann> to summarize. I don't think we should require passwords at creation time, I don't think we should ask for things we don't require, I don't think we should put vms on the network by default
So next I'll work on 'not requiring password for fedora' and remove password option all together but these patches should already go in as they do fix a regression.
(In reply to comment #14) > So next I'll work on 'not requiring password for fedora' and remove password > option all together but these patches should already go in as they do fix a > regression. No they don't "fix" it, fixing the regression would mean going back to the previous behaviour. They change behaviour so that the thing that regressed no longer happens.
(In reply to comment #15) > (In reply to comment #14) > > So next I'll work on 'not requiring password for fedora' and remove password > > option all together but these patches should already go in as they do fix a > > regression. > > No they don't "fix" it, fixing the regression would mean going back to the > previous behaviour. They change behaviour so that the thing that regressed no > longer happens. Fixing regression does not mean reverting to the old behaviour. It means fixing the issue caused (unintentionally). Do you see any problems with the patches?
(In reply to comment #16) > Fixing regression does not mean reverting to the old behaviour. It means fixing > the issue caused (unintentionally). Do you see any problems with the patches? Given that this does not really improve things with respect to telling the user what's going on (before, error message with no indication of what's wrong, after your patch, no error message, and no indication of what's wrong either), I think the current situation is better than what your patch would achieve, so I'd rather merge it at the same time as the changes the designers have suggested.
(In reply to comment #17) > (In reply to comment #16) > > Fixing regression does not mean reverting to the old behaviour. It means fixing > > the issue caused (unintentionally). Do you see any problems with the patches? > > Given that this does not really improve things with respect to telling the user > what's going on (before, error message with no indication of what's wrong, > after your patch, no error message, and no indication of what's wrong either), I totally disagree as the error message you are getting currently does not inform the user if setup failed because of her mistake or some system failure. With these patches, its pretty obvious that there is some info missing and therefore can't hit 'continue'. Having said that, I'll be providing an altnernate patch now that makes password optional for fedora as well and another patch that turns product key into a text field to indicate that its a required field. Also the other patch will need rebasing..
(In reply to comment #18) > With these patches, its pretty obvious that there is some info missing and > therefore can't hit 'continue'. > It's obvious that something is preventing to press 'continue', but it's not obvious *why* continue can't be pressed. Bug in Boxes? What kind of bug? Invalid ISO? Something wrong done by the user? I really think both are equally obscure.
(In reply to comment #19) > (In reply to comment #18) > > With these patches, its pretty obvious that there is some info missing and > > therefore can't hit 'continue'. > > > > It's obvious that something is preventing to press 'continue', but it's not > obvious *why* continue can't be pressed. Bug in Boxes? What kind of bug? > Invalid ISO? Something wrong done by the user? I really think both are equally > obscure. Well, I tried to explain the difference and don't know any better way to explain it better so I'll give up and we'll have to agree to disagree..
Created attachment 220684 [details] [review] express,fedora: Don't require password If no password is provided: * Set random password for admin * Setup passwordless user account
Created attachment 220685 [details] [review] wizard: Keep user in 'setup' until all data is provided We used to fail with an appropriate error message when user tried to hit 'continue' in setup page without providing needed data. Recently we had a regression in that regard so user was only getting a "VM setup failed" error. This patch fixes the regression but with an alternative solution: Keep the 'continue' button disabled as long as user hasn't provided all the needed data. Also now that fedora express install doesn't require password, username is the only field that is mandatory now.
Created attachment 220686 [details] [review] winxp,express: Require user to input product key Also remove the 'Add Product Key' button and show the entry by default to make the mandatory nature more obvious.
There was some more discussion about this on IRC that I forgot to paste here: <zeenix> [03:59:27] mccann: https://github.com/gnome-design-team/gnome-mockups/raw/master/boxes/boxes-install4.png <zeenix> [03:59:48] how will this look like w/o password? <mccann> [04:00:19] that mockup doesn't have a password field <zeenix> [04:00:40] ? <zeenix> [04:00:47] "Add a password" <mccann> [04:01:03] which isn't an entry <zeenix> [04:01:19] its an entry once you click on it <zeenix> [04:01:25] its optional <zeenix> [04:01:48] that is what i was talking about when i said "password is optional" <zeenix> [04:02:51] i understood that you wanted me to remove that <zeenix> [04:03:00] so we are back to my original question: <zeenix> how do we indicate which data on 'setup' page is required? <mccann> [04:03:44] the ones that have entries <mccann> [04:04:26] just username <mccann> [04:04:56] But I don't think we need the password thing there at all anymore
Review of attachment 220684 [details] [review]: ::: data/winxp.sif @@ +16,2 @@ [GuiUnattended] + AdminPassword="BOXES_ADMIN_PASSWORD" (patch needs to be rebased) How can the user get admin privileges if the password is randomly generated? I am not a Windows expert, but it seems like on WinXP at least, that is impossible. Since On Win7 the user is admin, he may access admin privileges (but not account). Anyway, I have no idea, and I am still unsure about this approach. So I can't really ack it
Review of attachment 220685 [details] [review]: ::: src/unattended-installer.vala @@ +312,3 @@ + if (mandatory) + entry.notify["text"].connect (() => { + run_in_thread (() => { run_in_thread(), why? You could make the code simpler by adding a virtual check_valid_user_data (). It shouldn't notify user-data-for-vm-creation-available change everytime the data is updated, but only when it is actually valid or not. notify is done for you when setting the property.
(In reply to comment #26) > Review of attachment 220684 [details] [review]: > > ::: data/winxp.sif > @@ +16,2 @@ > [GuiUnattended] > + AdminPassword="BOXES_ADMIN_PASSWORD" > > (patch needs to be rebased) Yeah, it also needs to be split as some unrelated changes got it.. > How can the user get admin privileges if the password is randomly generated? User is added to 'wheel' group so he/she can even easily change the root password if they want using 'sudo' etc. > I am not a Windows expert, but it seems like on WinXP at least, that is > impossible. > > Since On Win7 the user is admin, he may access admin privileges (but not > account). We are only using random password for fedora, for the rest we have always used same password for both admin and user (i-e no password required if none specified at 'setup' page). Fedora express install requires 'rootpw' command to be specified so we must set some password in there. While writing this, I realized how we can workaround this issue by unsetting the root password later in postinstall section so I'll try that..
(In reply to comment #27) > Review of attachment 220685 [details] [review]: > > ::: src/unattended-installer.vala > @@ +312,3 @@ > + if (mandatory) > + entry.notify["text"].connect (() => { > + run_in_thread (() => { > > run_in_thread(), why? > > You could make the code simpler by adding a virtual check_valid_user_data (). > It shouldn't notify user-data-for-vm-creation-available change everytime the > data is updated, but only when it is actually valid or not. notify is done for > you when setting the property. We are not checking for validity of data really so if we do, that will make the code more complicated rather than simpler. I guess i can make the code simpler by turning 'user-data-for-vm-creation-available' into an automatic prop and setting it appropriately rather than sending notifies..
(In reply to comment #29) > > run_in_thread(), why? > > > > You could make the code simpler by adding a virtual check_valid_user_data (). > > It shouldn't notify user-data-for-vm-creation-available change everytime the > > data is updated, but only when it is actually valid or not. notify is done for > > you when setting the property. > > We are not checking for validity of data really so if we do, that will make the > code more complicated rather than simpler. I guess i can make the code simpler > by turning 'user-data-for-vm-creation-available' into an automatic prop and > setting it appropriately rather than sending notifies.. yes, using automatic prop (and remove run_in_thread()) Well, it does some kind of checking. I mean you check the length of product key for example.
Created attachment 220885 [details] [review] express: Don't set SPICE password Our own created VMs are supposed to be private to user (at least by default) so there is no need to add password protection to display connection.
Created attachment 220886 [details] [review] express,fedora: Don't use remote repos for F16 Now that F17 has been out for a while and F18 isn't that far off, F16 isn't important enough anymore to justify (very slow) installation from network and the synchronous network availability check. Better just not install buggy vdagent/QXL.
Created attachment 220887 [details] [review] express,fedora: Don't require password We now make use of kickstart file's post installation setup to manually setup user and root passwords. This makes it possible for us to unset passwords when needed (user doesn't specify password in 'setup'). This makes Fedora's password policy consistent with Windows XP and 7.
(In reply to comment #32) > Created an attachment (id=220886) [details] [review] > express,fedora: Don't use remote repos for F16 > > Now that F17 has been out for a while and F18 isn't that far off, F16 > isn't important enough anymore to justify (very slow) installation from > network and the synchronous network availability check. Better just not > install buggy vdagent/QXL. This code is only hit when an f16 iso is detected, isn't it? With f16 still being supported, I'd keep this code until it's EOL (1 month after f18 release)
Created attachment 220888 [details] [review] wizard: Keep user in 'setup' until all data is provided V2: I tried to modify this to use automatic properties and not send notify needlessly but: * Code was actually getting more complicated that way as then you'll have to keep track of all the other conditions when one condition changes (e.g if express install is toggled, do we have all the data needed?). * Use of automatic properties doesn't currently ensure we do not send notify when value remains unchanged: https://bugzilla.gnome.org/show_bug.cgi?id=631267 * According to GObject::notify docs, a notify doesn't imply an actual change so its quite alright to notify even when there is no real change in prop value. I did remove the run_in_thread() though. It was a hack!
Created attachment 220889 [details] [review] winxp,express: Require user to input product key Also remove the 'Add Product Key' button and show the entry by default to make the mandatory nature more obvious.
(In reply to comment #34) > (In reply to comment #32) > > Created an attachment (id=220886) [details] [review] [details] [review] > > express,fedora: Don't use remote repos for F16 > > > > Now that F17 has been out for a while and F18 isn't that far off, F16 > > isn't important enough anymore to justify (very slow) installation from > > network and the synchronous network availability check. Better just not > > install buggy vdagent/QXL. > > This code is only hit when an f16 iso is detected, isn't it? Should, yes :) > With f16 still > being supported, I'd keep this code until it's EOL (1 month after f18 release) Well, this change is going to 3.6 branch (i-e meant for F18) and by the time F16 reaches EOL, we shouldn't be making any changes to 3.6 branch except for bug fixes.
(In reply to comment #37) > > > With f16 still > > being supported, I'd keep this code until it's EOL (1 month after f18 release) > > Well, this change is going to 3.6 branch (i-e meant for F18) and by the time > F16 reaches EOL, we shouldn't be making any changes to 3.6 branch except for > bug fixes. Imo this doesn't hurt to keep it until 3.7/3.8.
Review of attachment 220885 [details] [review]: ack
Review of attachment 220886 [details] [review]: ack
Review of attachment 220887 [details] [review]: ack
Review of attachment 220888 [details] [review]: ack
Review of attachment 220889 [details] [review]: ack
Attachment 220885 [details] pushed as 433afad - express: Don't set SPICE password Attachment 220886 [details] pushed as 169a4f5 - express,fedora: Don't use remote repos for F16 Attachment 220887 [details] pushed as 77034f1 - express,fedora: Don't require password Attachment 220888 [details] pushed as 2b22dfe - wizard: Keep user in 'setup' until all data is provided Attachment 220889 [details] pushed as c27e6de - winxp,express: Require user to input product key
(In reply to comment #44) > Attachment 220886 [details] pushed as 169a4f5 - express,fedora: Don't use remote repos for F16 Woo, thanks for totally ignoring my concerns /o\
<mccann> to summarize. I don't think we should require passwords at creation time, I don't think we should ask for things we don't require, I don't think we should put vms on the network by default Keeping this open as I don't think this has been addressed.
(In reply to comment #45) > (In reply to comment #44) > > Attachment 220886 [details] [details] pushed as 169a4f5 - express,fedora: Don't use remote repos for F16 > > Woo, thanks for totally ignoring my concerns /o\ I replied to that so 'totally ignoring' is quite an exageration. I won't always be able to satisfy everyone so if I thought the change is good and I get an ack from a Boxes developer/maintainer, I'll commit the change. Really have better things to do than to discuss every small change to death. (In reply to comment #46) > <mccann> to summarize. I don't think we should require passwords at creation > time, I don't think we should ask for things we don't require, I don't think we > should put vms on the network by default > > Keeping this open as I don't think this has been addressed. The 'we should not put VMs on network by default' is not exaclty this bug so feel free to create one for that. We are not requiring password anymore for fedora so that has been addressed.
(In reply to comment #47) > (In reply to comment #45) > > (In reply to comment #44) > > > Attachment 220886 [details] [details] [details] pushed as 169a4f5 - express,fedora: Don't use remote repos for F16 > > > > Woo, thanks for totally ignoring my concerns /o\ > > I replied to that so 'totally ignoring' is quite an exageration. I won't always > be able to satisfy everyone so if I thought the change is good and I get an ack > from a Boxes developer/maintainer, I'll commit the change. Really have better > things to do than to discuss every small change to death. Well, so far what I've seen is "I've written this patch, so this will go in, I'll ignore you". No clue *why* we have to introduce a regression on a supported distribution except that you decided that it had to be done. > > (In reply to comment #46) > > <mccann> to summarize. I don't think we should require passwords at creation > > time, I don't think we should ask for things we don't require, I don't think we > > should put vms on the network by default > > > > Keeping this open as I don't think this has been addressed. > > The 'we should not put VMs on network by default' is not exaclty this bug so > feel free to create one for that. We are not requiring password anymore for > fedora so that has been addressed. There are 3 items in this sentence, you missed the middle one.
(In reply to comment #48) > (In reply to comment #47) > > (In reply to comment #45) > > > (In reply to comment #44) > > > > Attachment 220886 [details] [details] [details] [details] pushed as 169a4f5 - express,fedora: Don't use remote repos for F16 > > > > > > Woo, thanks for totally ignoring my concerns /o\ > > > > I replied to that so 'totally ignoring' is quite an exageration. I won't always > > be able to satisfy everyone so if I thought the change is good and I get an ack > > from a Boxes developer/maintainer, I'll commit the change. Really have better > > things to do than to discuss every small change to death. > > Well, so far what I've seen is "I've written this patch, so this will go in, > I'll ignore you". No clue *why* we have to introduce a regression on a > supported distribution except that you decided that it had to be done. What regression? Thats also an exageration. We are just not going to install/have spice goodies on F16 but OTOH the installation will be *much* faster and will not require network. This was a hack to workaround the issue of fedora guys not updating ISOs after release even for critical fixes and it only made sense when F16 was new. > > (In reply to comment #46) > > > <mccann> to summarize. I don't think we should require passwords at creation > > > time, I don't think we should ask for things we don't require, I don't think we > > > should put vms on the network by default > > > > > > Keeping this open as I don't think this has been addressed. > > > > The 'we should not put VMs on network by default' is not exaclty this bug so > > feel free to create one for that. We are not requiring password anymore for > > fedora so that has been addressed. > > There are 3 items in this sentence, you missed the middle one. I didn't! Following this with mccann, I understood that he is not sure about password being there and I asked him to come-up with new mockups if he wants us to remove password all together.
(In reply to comment #49) > What regression? Thats also an exageration. f16 will not work as good as before, this is what I call a regression. > We are just not going to > install/have spice goodies on F16 but OTOH the installation will be *much* > faster and will not require network. This was a hack to workaround the issue of > fedora guys not updating ISOs after release even for critical fixes and it only > made sense when F16 was new. I don't see why it no longer makes sense now... And I still see no objective reason for removing that code now, rather than after f16 is EOL'ed. f16 is old, why do we need to change the behaviour of what we install? Have you tested these changes? I just made a f16 install with these changes and the after installation the buggy qxl driver is used. And this is something I would have brought up during a proper review instead if you were interested in addressing other people's comments rather then getting your patches in as quickly as possible... > > > > (In reply to comment #46) > > > > <mccann> to summarize. I don't think we should require passwords at creation > > > > time, I don't think we should ask for things we don't require, I don't think we > > > > should put vms on the network by default > > > > > > > > Keeping this open as I don't think this has been addressed. > > > > > > The 'we should not put VMs on network by default' is not exaclty this bug so > > > feel free to create one for that. We are not requiring password anymore for > > > fedora so that has been addressed. > > > > There are 3 items in this sentence, you missed the middle one. > > I didn't! Following this with mccann, I understood that he is not sure about > password being there and I asked him to come-up with new mockups if he wants us > to remove password all together. ah, and where is that mentioned in https://bugzilla.gnome.org/show_bug.cgi?id=680825#c47 ? You didn't address the middle item in this comment, that's what I meant. The information you just gave would have been useful there.
All of this is addressed as far as I can tell.