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

Fix crash when devices and connections are changing (#1245960) #274

Closed

Conversation

jkonecny12
Copy link
Member

First when user add new device to started anaconda in Welcome screen then
the anaconda will crash in NetworkSpoke initialization. This happens
because it's not created configuration file for the new device.
Fixed by generating this configuration in NetworkSpoke initialization.

Second when user remove existing ethernet connection for device. This could
happen specially in Live compose. This is fixed now that we skipping
devices without connection and add that device after the connection
appears.

Resolves: rhbz#1245960

@jkonecny12 jkonecny12 added the master Please, use the `f39` label instead. label Aug 4, 2015
# Configs for ethernet has been already added,
# this must be some slave
if dev_cfg.device_type == NetworkManager.DeviceType.ETHERNET:
# skip slaves - only slaves has master
Copy link
Contributor

Choose a reason for hiding this comment

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

have^

Copy link
Member Author

Choose a reason for hiding this comment

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

ahh thank you. I'll change this locally

@vpodzime
Copy link
Contributor

vpodzime commented Aug 4, 2015

Other than the nitpick above this looks good to me.

@vpodzime vpodzime added the ACK label Aug 4, 2015
if dev_cfg:
dev_cfg.device = device
else:
# it's wireless or device with missing connection
# skip device with missing connection now, will be add when connection appears
Copy link
Contributor

Choose a reason for hiding this comment

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

'skip the device'
'will be added'

@bcl
Copy link
Contributor

bcl commented Aug 5, 2015

Looks ok to me, other than those subtle little grammar changes.

@jkonecny12
Copy link
Member Author

Ahh thank you @bcl for the grammar correction.
Sorry for that I have to look on a commit in more detail next time.

@rvykydal
Copy link
Contributor

rvykydal commented Aug 5, 2015

I think we should find the NM device object and set device attribute of DeviceConfiguration when creating it in add_connection_to_list() for ETHERNET device case (ie the case when ethernet device is added). It seems the code assumes the DeviceConfiguration.device to be present for eth devices.

Also we should connect callbacks for the device in add_device_to_list, which does not happen due to earlier return before
device.connect("notify::ip4-config", self.on_device_config_changed)
device.connect("notify::ip6-config", self.on_device_config_changed)
device.connect("state-changed", self.on_device_state_changed)
perhaps instead of the return we should just move #wireless device into else: branch

@jkonecny12 jkonecny12 removed the ACK label Aug 6, 2015
Anaconda will crash when user remove existing ethernet connection
for the device. This could happen especially in Live compose.
Fixed now by skipping devices without connection and add that device
after the connection appears.

Related: rhbz#1245960
Anaconda will crash when user add a new device in the Welcome screen.
This happens because the configuration file for the new device wasn't
created.
Fixed by generating this configuration in NetworkSpoke initialization.

Resolves: rhbz#1245960
@jkonecny12 jkonecny12 force-pushed the master-settings-not-found-usb branch from 31a8cb6 to c1d6d59 Compare August 11, 2015 12:34
@jkonecny12
Copy link
Member Author

Updated PR.
I fixed the grammar and applied the @rvykydal changes.
I also split the commit to two because it's fixing two problems.

self.add_dev_cfg(dev_cfg)
log.debug("network: GUI, adding connection %s", uuid)
return True

def initialize(self):
# There is a signal for newly added devices from NetworkManager but it
# is registered after the initialize method.
# So if someone adds a new device in the Welcome screen the ifconf file won't
Copy link
Contributor

Choose a reason for hiding this comment

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

...the ifcfg file... not ifconf file I believe.

@vpodzime
Copy link
Contributor

Looks good to me other than the nitpick above. But I'm leaving this to @rvykydal for a review as he knows these "waters" much better. :)

@jkonecny12
Copy link
Member Author

Thank you I'll change it locally.

@rvykydal rvykydal added the ACK label Aug 12, 2015
@jkonecny12
Copy link
Member Author

Pushed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master Please, use the `f39` label instead.
4 participants