[PATCH 2/3] Add the s390 plugin functions.

Vratislav Podzimek vpodzime at redhat.com
Tue May 19 11:37:25 UTC 2015


On Mon, 2015-05-18 at 15:10 -0400, Samantha N. Bueno wrote:
> This includes the header, api, and source for all s390-related
> functions, as well as the Python bindings.
> ---
>  dist/libblockdev.spec               |  32 +++
>  src/lib/plugin_apis/s390.api        |  78 +++++++
>  src/plugins/s390.c                  | 414 ++++++++++++++++++++++++++++++++++++
>  src/plugins/s390.h                  |  79 +++++++
>  src/python/gi/overrides/BlockDev.py |  50 +++++
>  5 files changed, 653 insertions(+)
>  create mode 100644 src/lib/plugin_apis/s390.api
>  create mode 100644 src/plugins/s390.c
>  create mode 100644 src/plugins/s390.h
> 
> diff --git a/dist/libblockdev.spec b/dist/libblockdev.spec
> index 47f1c47..73f2268 100644
> --- a/dist/libblockdev.spec
> +++ b/dist/libblockdev.spec
> @@ -217,6 +217,25 @@ Requires: glib2-devel
>  This package contains header files and pkg-config files needed for development
>  with the libblockdev-swap plugin/library.
>  
> +%package s390
> +Summary:    The s390 plugin for the libblockdev library
> +Requires: s390utils
> +
> +%description s390
> +The libblockdev library plugin (and in the same time a standalone library)
> +providing the functionality related to s390 devices.
> +
> +%package s390-devel
> +Summary:     Development files for the libblockdev-s390 plugin/library
> +Requires: %{name}-s390%{?_isa} = %{version}-%{release}
> +Requires: %{name}-utils-devel%{?_isa}
> +Requires: glib2-devel
> +Requires: s390utils-devel
> +
> +%description s390-devel
> +This package contains header files and pkg-config files needed for development
> +with the libblockdev-s390 plugin/library.
> +
>  
>  %package plugins-all
>  Summary:     Meta-package that pulls all the libblockdev plugins as dependencies
> @@ -229,6 +248,9 @@ Requires: %{name}-lvm%{?_isa} = %{version}-%{release}
>  Requires: %{name}-mdraid%{?_isa} = %{version}-%{release}
>  Requires: %{name}-mpath%{?_isa} = %{version}-%{release}
>  Requires: %{name}-swap%{?_isa} = %{version}-%{release}
> +%ifarch s390 s390x
> +Requires: %{name}-s390%{?_isa} = %{version}-%{release}
> +%endif
>  
>  %description plugins-all
>  A meta-package that pulls all the libblockdev plugins as dependencies.
> @@ -264,6 +286,8 @@ CFLAGS="%{optflags}" make PREFIX=%{buildroot} SITEDIRS=%{buildroot}%{python2_sit
>  %postun mpath -p /sbin/ldconfig
>  %post swap -p /sbin/ldconfig
>  %postun swap -p /sbin/ldconfig
> +%post s390 -p /sbin/ldconfig
> +%postun s390 -p /sbin/ldconfig
>  
> 
>  %files
> @@ -368,6 +392,14 @@ CFLAGS="%{optflags}" make PREFIX=%{buildroot} SITEDIRS=%{buildroot}%{python2_sit
>  %dir %{_includedir}/blockdev
>  %{_includedir}/blockdev/swap.h
>  
> +%files s390
> +%{_libdir}/libbd_s390.so.*
> +
> +%files s390-devel
> +%{_libdir}/libbd_s390.so
> +%dir %{_includedir}/blockdev
> +%{_includedir}/blockdev/s390.h
> +
>  %files plugins-all
>  
>  %changelog
> diff --git a/src/lib/plugin_apis/s390.api b/src/lib/plugin_apis/s390.api
> new file mode 100644
> index 0000000..54984fe
> --- /dev/null
> +++ b/src/lib/plugin_apis/s390.api
> @@ -0,0 +1,78 @@
> +#include <glib.h>
> +#include <linux/fs.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <utils.h>
> +#include <asm/dasd.h>
> +#include <s390utils/vtoc.h>
> +
> +
> +typedef enum {
> +    BD_S390_ERROR_DEVICE,
> +    BD_S390_ERROR_FORMAT_FAILED,
> +    BD_S390_ERROR_PARSE,
> +} BDS390Error;
> +
> +
> +/**
> + * bd_s390_dasd_format:
> + * @dasd: dasd to format
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: whether dasdfmt was successful or not
> + */
> +gboolean bd_s390_dasd_format (gchar *dasd, GError **error);
> +
> +/**
> + * bd_s390_dasd_needs_format:
> + * @dasd: dasd to check, whether it needs dasdfmt run on it
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: whether a dasd needs dasdfmt run against it
> + */
> +gboolean bd_s390_dasd_needs_format (gchar *dasd, GError **error);
> +
> +/**
> + * bd_s390_dasd_online:
> + * @dasd: dasd to switch online
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: whether a dasd was successfully switched online
> + */
> +gboolean bd_s390_dasd_online (gchar *dasd, GError **error);
> +
> +/**
> + * bd_s390_dasd_is_Ldl:
> + * @dasd: dasd to check, whether it is LDL formatted
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: whether a dasd is LDL formatted
> + */
> +gboolean bd_s390_dasd_is_ldl (gchar *dasd, GError **error);
> +
> +/**
> + * bd_s390_sanitize_dev_input:
> + * @dev a DASD or zFCP device number
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: a synthesized dasd or zfcp device number
> + */
> +gchar* bd_s390_sanitize_dev_input (gchar *dev, GError **error);
> +
> +/**
> + * bd_s390_zfcp_sanitize_wwpn_input:
> + * @dev a zFCP WWPN identifier
Mismatch in the name of the parameter.

> + * @error: (out): place to store error (if any)
> + *
> + * Returns: a synthesized zFCP WWPN
> + */
> +gchar* bd_s390_zfcp_sanitize_wwpn_input (gchar *wwpn, GError **error);
> +
> +/**
> + * bd_s390_zfcp_sanitize_lun_input:
> + * @dev a zFCP LUN identifier
Same here.

> + * @error: (out): place to store error (if any)
> + *
> + * Returns: a synthesized zFCP LUN
> + */
> +gchar* bd_s390_zfcp_sanitize_lun_input (gchar *lun, GError **error);
> diff --git a/src/plugins/s390.c b/src/plugins/s390.c
> new file mode 100644
> index 0000000..6cea145
> --- /dev/null
> +++ b/src/plugins/s390.c
> @@ -0,0 +1,414 @@
> +/*
> + * Copyright (C) 2015  Red Hat, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Author: Samantha N. Bueno <sbueno at redhat.com>
> + */
> +
> +#include <glib.h>
> +#include <linux/fs.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <utils.h>
> +#include <asm/dasd.h>
> +#include <s390utils/vtoc.h>
> +
> +#include "s390.h"
> +
> +
> +/**
> + * SECTION: s390
> + * @short_description: plugin for operations with s390
> + * @title: s390
> + * @include: s390.h
> + *
> + * A plugin for operations with s390 devices.
> + */
> +
> +/**
> + * bd_s390_error_quark: (skip)
> + */
> +GQuark bd_s390_error_quark (void) {
> +    return g_quark_from_static_string ("g-bd-s390-error-quark");
> +}
> +
> +
> +/**
> + * check: (skip)
> + */
> +gboolean check() {
> +    GError *error = NULL;
> +    /* dasdfmt doesn't return version info */
> +    gboolean ret = bd_utils_check_util_version ("dasdfmt", NULL, NULL, NULL, &error);
> +    if (!ret && error) {
> +        g_warning("Cannot load the s390 plugin: %s" , error->message);
> +        g_clear_error (&error);
> +    }
> +    return ret;
> +}
> +
> +/**
> + * bd_s390_dasd_format:
> + * @dasd: dasd to format
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: whether dasdfmt was successful or not
> + */
> +gboolean bd_s390_dasd_format (gchar *dasd, GError **error) {
> +    gboolean rc = FALSE;
> +    gchar *dev = g_strdup_printf ("/dev/%s", dasd);
> +    gchar *argv[] = {"/sbin/dasdfmt", "-y", "-d", "cdl", "-b", "4096", dev, NULL};
> +
> +    rc = bd_utils_exec_and_report_error (argv, error);
> +    g_free (dev);
> +    return rc;
> +}
> +
> +/**
> + * bd_s390_dasd_needs_format:
> + * @dasd: dasd to check, given as device number
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: whether a dasd needs dasdfmt run against it
> + */
> +gboolean bd_s390_dasd_needs_format (gchar *dasd, GError **error) {
> +    gchar status[12];
> +    gchar *path = NULL;
> +    gchar *rc = NULL;
> +    FILE *fd = NULL;
> +
> +    path = g_strdup_printf ("/sys/bus/ccw/drivers/dasd-eckd/%s/status", dasd);
> +    fd = fopen(path, "r");
> +    g_free (path);
> +    if (!fd) {
> +        g_set_error (error, BD_S390_ERROR, BD_S390_ERROR_DEVICE, "Error checking status of device %s; device may not exist, or status can not be read.", dasd);
Please move the long string above to the next line.

> +        g_clear_error (error);
> +        return FALSE;
> +    }
> +
> +    rc = fgets (status, 12, fd);
You should do 
  status[11] = '\0';
here to make sure there is a null-byte at the very end (as you treat the
buffer as a string below). What are the other possible contents of the
file?

> +    fclose(fd);
> +
> +    if (!rc) {
> +        g_set_error (error, BD_S390_ERROR, BD_S390_ERROR_DEVICE, "Error checking status of device %s.", dasd);
> +        g_clear_error (error);
> +        return FALSE;
> +    }
> +
> +    if (g_ascii_strncasecmp (status, "unformatted", strlen(status)) == 0) {
Here you rely on 'status' being a string (null-terminated) since you try
to get it's length. Using 11 (as the length of the static string
"unformatted") should work the same way here.

> +        g_warning ("Device %s status is %s, needs dasdfmt.", dasd, status);
> +        g_clear_error (error);
> +        return TRUE;
> +    }
> +
> +    return FALSE;
> +}
> +
> +/**
> + * bd_s390_dasd_online:
> + * @dasd: dasd to switch online, given as device number
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: whether a dasd was successfully switched online
> + */
> +gboolean bd_s390_dasd_online (gchar *dasd, GError **error) {
> +    gboolean rc = FALSE;
> +    gint wrc = 0;
> +    gint online = 0;
> +    gchar *path = NULL;
> +    FILE *fd = NULL;
> +    gchar *argv[] = {"/usr/sbin/dasd_cio_free", "-d", dasd, NULL};
> +
> +    path = g_strdup_printf ("/sys/bus/ccw/drivers/dasd-eckd/%s/online", dasd);
> +    fd = fopen(path, "r+");
> +    if (!fd) {
> +        /* DASD might be in device ignore list; try to rm it */
> +        rc = bd_utils_exec_and_report_error (argv, error);
> +
> +        if (rc) {
> +            return rc;
> +        }
Should this really return if the 'dasd_cio_free' succeeds?

> +        /* fd is NULL at this point, so try to open it */
> +        fd = fopen(path, "r+");
> +        g_free (path);
> +
> +        if (!fd) {
> +            g_set_error (error, BD_S390_ERROR, BD_S390_ERROR_DEVICE, "Could not open device %s even after removing it from the device ignore list.", dasd);
Again a long string that should go to a separate line.

> +            g_clear_error (error);
> +            return FALSE;
> +        }
> +    }
'path' should be free'd in an else branch here.

> +
> +    /* check whether our DASD is online; if not, set it */
> +    online = fgetc(fd);
> +
> +    if (online == EOF) {
> +        /* there was some error checking the 'online' status */
> +        g_set_error (error, BD_S390_ERROR, BD_S390_ERROR_DEVICE, "Error checking if device %s is online", dasd);
> +        g_clear_error (error);
You probably don't want to set error here and clear it immediately.

> +        fclose(fd);
> +        return FALSE;
> +    }
> +    if (online == 1) {
> +        g_set_error (error, BD_S390_ERROR, BD_S390_ERROR_DEVICE, "DASD device %s is already online.", dasd);
> +        g_clear_error (error);
Same here.

> +        fclose(fd);
> +        return FALSE;
> +    }
> +    else {
> +        /* reset file cursor before writing to it */
> +        rewind (fd);
> +        wrc = fputs("1", fd);
> +    }
> +
> +    fclose(fd);
> +
> +    if (wrc == EOF) {
> +        g_warning ("Could not set DASD device %s online", dasd);
> +        fclose(fd);
> +        g_clear_error (error);
'error' is/may not be set here, there's no reason to clear it.

> +        return FALSE;
> +    }
> +
> +    return TRUE;
> +}
> +
> +/**
> + * bd_s390_dasd_read_vlabel:
> + * @dasd_info:
> + * @fd: a file descriptor for the open DASD device
> + * @blksize:
> + * @vlabel:
> + *
> + * Returns:
Wrong function name in the header and missing docs. Feel free to remove
them entirely as this is an internal function anyway.

> + */
> +static int dasd_read_vlabel(dasd_information2_t *dasd_info, int fd, int blksize, volume_label_t *vlabel) {
> +    gint rc;
> +    unsigned long vlabel_start;
> +    vlabel_start = dasd_info->label_block * blksize;
> +
> +    memset(vlabel, 0, sizeof(volume_label_t));
> +
> +    if (lseek(fd, vlabel_start, SEEK_SET) < 0) {
> +        return 2;
> +    }
> +
> +    rc = read(fd, vlabel, sizeof(volume_label_t));
> +    if (rc != sizeof(volume_label_t)) {
> +        return 1;
> +    }
The fact that read() doesn't read the number of bytes requested doesn't
necessarily mean that there has been an error (it, for example, might
have been interrupted by a signal). You should check rc for being -1
(ERROR) and if it's less than sizeof(volume_label_t), you should try
reading more until you get -1 or 0 at which point either enough was read
(OK) or there's nothing more to read (ERROR).
Or you could switch to GLib's GIOChannel that, AFAICT, handles that
internally (just make sure to set encoding to NULL, default is utf-8)

> +
> +    return 0;
> +}
> +
> +/**
> + * bd_s390_dasd_is_ldl:
> + * @dasd: dasd to check, whether it is LDL formatted
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: whether a dasd is LDL formatted
> + */
> +gboolean bd_s390_dasd_is_ldl (gchar *dasd, GError **error) {
> +    gchar *devname = NULL;
> +    gint f = 0;
> +    gint ret = 0;
> +    gint blksize = 0;
> +    dasd_information2_t dasd_info;
> +    volume_label_t vlabel;
> +
> +    memset(&dasd_info, 0, sizeof(dasd_info));
> +    devname = g_strdup_printf ("/dev/%s", dasd);
> +
> +    if ((f = open(devname, O_RDONLY)) == -1) {
> +        g_set_error (error, BD_S390_ERROR, BD_S390_ERROR_DEVICE, "Unable to open device %s", devname);
> +        g_free (devname);
> +        return FALSE;
> +    }
> +    if (ioctl(f, BLKSSZGET, &blksize) != 0) {
> +        close(f);
> +        return FALSE;
> +    }
> +
> +    if (ioctl(f, BIODASDINFO2, &dasd_info) != 0) {
> +        close(f);
> +        return FALSE;
> +    }
> +
> +    ret = dasd_read_vlabel(&dasd_info, f, blksize, &vlabel);
The 'vlabel' doesn't seem to be used anywhere below.

> +    close(f);
> +    if (ret == 2) {
> +        return FALSE;
> +    }
What about 'ret == 1'? Maybe check '!= 0' here instead?

> +
> +    if (strncmp(dasd_info.type, "FBA", 3) == 0) {
> +        return FALSE;
> +    }
> +
> +    /* check dasd volume label; "VOL1" is a CDL formatted DASD, won't require formatting */
Should use 'vlabel' somewhere, maybe?

> +    if (dasd_info.format == DASD_FORMAT_CDL) {
> +        return FALSE;
> +    }
> +    else {
> +        return TRUE;
> +    }
> +}
> +
> +/**
> + * bd_s390_sanitize_dev_input:
> + * @dev a DASD or zFCP device number
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: (transfer full): a synthesized dasd or zfcp device number
> + */
> +gchar* bd_s390_sanitize_dev_input (gchar *dev, GError **error) {
> +    gchar *tok = NULL;
> +    gchar *tmptok = NULL;
> +    gchar *prepend = NULL;
> +    gchar *ret = NULL;
> +    gchar *lcdev = NULL;
> +
> +    /* first make sure we're not being played */
> +    if ((dev == NULL) || (!*dev)) {
> +        g_set_error (error, BD_S390_ERROR, BD_S390_ERROR_DEVICE, "You have not specified a device number or the number is invalid");
> +        g_clear_error (error);
> +        return "";
> +    }
> +
> +    /* convert everything to lowercase */
> +    lcdev = g_ascii_strdown (dev, strlen(dev));
You can use -1 as the second parameter as 'dev' is a nul-terminated
string.

> +
> +    /* we only care about the last piece of the device number, since
> +     * that's the only part which will need formatting */
> +    tok = g_strrstr(lcdev, ".");
> +
> +    if (tok) {
> +        tmptok = tok + 1; /* want to ignore the delimiter char */
> +    }
> +    else {
> +        tmptok = lcdev;
> +    }
> +
> +    /* calculate if/how much to pad tok with */
> +    prepend = g_strnfill((4 - strlen(tmptok)), '0');
> +
> +    /* combine it all together */
> +    if (prepend == NULL) {
> +        ret = g_strdup_printf("0.0.%s", tmptok);
> +    }
> +    else {
> +        ret = g_strdup_printf("0.0.%s%s", prepend, tmptok);
> +    }
> +    g_free (prepend);
> +    g_free (lcdev);
> +
> +    return ret;
> +}
> +
> +/**
> + * bd_s390_zfcp_sanitize_wwpn_input:
> + * @dev a zFCP WWPN identifier
param name mismatch as in the .api file

> + * @error: (out): place to store error (if any)
> + *
> + * Returns: (transfer full): a synthesized zFCP WWPN
> + */
> +gchar* bd_s390_zfcp_sanitize_wwpn_input (gchar *wwpn, GError **error) {
> +    gchar *fullwwpn = NULL;
> +    gchar *lcwwpn = NULL;
> +
> +    /* first make sure we're not being played */
> +    if ((wwpn == NULL) || (!*wwpn)) {
> +        g_set_error (error, BD_S390_ERROR, BD_S390_ERROR_DEVICE, "You have not specified a WWPN or the WWPN is invalid");
> +        g_clear_error (error);
> +        return "";
> +    }
> +
> +    /* convert everything to lowercase */
> +    lcwwpn = g_ascii_strdown (wwpn, strlen(wwpn));
Same trick with -1 here as above and in more places below.

> +
> +    if (strncmp(lcwwpn, "0x", 2) == 0) {
> +        /* user entered a properly formatted wwpn */
> +        fullwwpn = g_strdup_printf("%s", lcwwpn);
> +    }
> +    else {
> +        fullwwpn = g_strdup_printf("0x%s", lcwwpn);
> +    }
> +    g_free (lcwwpn);
> +    return fullwwpn;
> +}
> +
> +/**
> + * bd_s390_zfcp_sanitize_lun_input:
> + * @dev a zFCP LUN identifier
param name mismatch as in the .api file

> + * @error: (out): place to store error (if any)
> + *
> + * Returns: (transfer full): a synthesized zFCP LUN
> + */
> +gchar* bd_s390_zfcp_sanitize_lun_input (gchar *lun, GError **error) {
> +    gchar *lclun = NULL;
> +    gchar *tmplun = NULL;
> +    gchar *fulllun = NULL;
> +    gchar *prepend = NULL;
> +    gchar *append = NULL;
> +
> +    /* first make sure we're not being played */
> +    if ((lun == NULL) || (!*lun)) {
> +        g_set_error (error, BD_S390_ERROR, BD_S390_ERROR_DEVICE, "You have not specified a LUN or the LUN is invalid");
> +        g_clear_error (error);
> +        return "";
> +    }
> +
> +    /* convert everything to lowercase */
> +    lclun = g_ascii_strdown (lun, strlen(lun));
> +
> +    if ((g_str_has_prefix(lclun, "0x")) && (strlen(lclun) == 18)) {
> +        /* user entered a properly formatted lun */
> +        fulllun = g_strdup_printf ("%s", lclun);
> +    }
> +    else {
> +        /* we need to mangle the input to make it proper. ugh. */
> +        if (g_str_has_prefix (lclun, "0x")) {
> +            /* this may seem odd, but it makes the math easier a ways down */
> +            tmplun = lclun + 2;
> +        }
> +        else {
> +            tmplun = lclun;
> +        }
> +
> +        if (strlen(tmplun) < 4) {
> +            /* check if/how many zeros we pad to the left */
> +            prepend = g_strnfill((4 - strlen(tmplun)), '0');
> +            /* check if/how many zeros we pad to the right */
> +            append = g_strnfill((16 - (strlen(tmplun) + strlen(prepend))), '0');
> +        }
> +        else {
> +            /* didn't need to pad anything on the left; so just check if/how
> +             * many zeros we pad to the right */
> +            append = g_strnfill((16 - (strlen(tmplun))), '0');
> +        }
> +
> +        /* now combine everything together */
> +        if (prepend == NULL) {
> +            fulllun = g_strdup_printf ("0x%s%s", tmplun, append);
> +        }
> +        else {
> +            fulllun = g_strdup_printf ("0x%s%s%s", prepend, tmplun, append);
> +        }
> +    }
> +    g_free (lclun);
> +    g_free (prepend);
> +    g_free (append);
> +
> +    return fulllun;
> +}
> diff --git a/src/plugins/s390.h b/src/plugins/s390.h
> new file mode 100644
> index 0000000..52cb3ab
> --- /dev/null
> +++ b/src/plugins/s390.h
> @@ -0,0 +1,79 @@
> +#include <glib.h>
> +#include <linux/fs.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <utils.h>
> +#include <asm/dasd.h>
> +#include <s390utils/vtoc.h>
> +
> +
> +GQuark bd_s390_error_quark (void);
> +#define BD_S390_ERROR bd_s390_error_quark ()
> +typedef enum {
> +    BD_S390_ERROR_DEVICE,
> +    BD_S390_ERROR_FORMAT_FAILED,
> +    BD_S390_ERROR_DASDFMT,
> +} BDS390Error;
> +
> +/**
> + * bd_s390_dasd_format:
> + * @dasd: dasd to format
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: whether dasdfmt was successful or not
> + */
> +gboolean bd_s390_dasd_format (gchar *dasd, GError **error);
> +
> +/**
> + * bd_s390_dasd_needs_format:
> + * @dasd: dasd to check, whether it needs dasdfmt run on it
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: whether a dasd needs dasdfmt run against it
> + */
> +gboolean bd_s390_dasd_needs_format (gchar *dasd, GError **error);
> +
> +/**
> + * bd_s390_sanitize_dev_input:
> + * @dev a DASD or zFCP device number
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: a synthesized dasd or zfcp device number
> + */
> +gchar* bd_s390_sanitize_dev_input (gchar *dev, GError **error);
> +
> +/**
> + * bd_s390_dasd_online:
> + * @dasd: dasd to switch online
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: whether a dasd was successfully switched online
> + */
> +gboolean bd_s390_dasd_online (gchar *dasd, GError **error);
> +
> +/**
> + * bd_s390_dasd_is_cdl:
> + * @dasd: dasd to check, whether it is CDL formatted
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: whether a dasd is CDL formatted
> + */
> +gboolean bd_s390_dasd_is_ldl (gchar *dasd, GError **error);
> +
> +/**
> + * bd_s390_zfcp_sanitize_wwpn_input:
> + * @dev a zFCP WWPN identifier
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: a synthesized zFCP WWPN
> + */
> +gchar* bd_s390_zfcp_sanitize_wwpn_input (gchar *wwpn, GError **error);
> +
> +/**
> + * bd_s390_zfcp_sanitize_lun_input:
> + * @dev a zFCP LUN identifier
> + * @error: (out): place to store error (if any)
> + *
> + * Returns: a synthesized zFCP LUN
> + */
> +gchar* bd_s390_zfcp_sanitize_lun_input (gchar *lun, GError **error);
The documentation strings should be dropped from the header file (they
confuse gtk-doc or something). We ship full HTML/XML documentation in
the packages as well as make it available online. That should be enough.

> diff --git a/src/python/gi/overrides/BlockDev.py b/src/python/gi/overrides/BlockDev.py
> index 5e47f03..404e5a8 100644
> --- a/src/python/gi/overrides/BlockDev.py
> +++ b/src/python/gi/overrides/BlockDev.py
> @@ -45,6 +45,7 @@ bd_plugins = { "lvm": BlockDev.Plugin.LVM,
>                 "swap": BlockDev.Plugin.SWAP,
>                 "mdraid": BlockDev.Plugin.MDRAID,
>                 "mpath": BlockDev.Plugin.MPATH,
> +               "s390": BlockDev.Plugin.S390,
>  }
>  
>  _init = BlockDev.init
> @@ -258,6 +259,48 @@ def swap_swapon(device, priority=-1):
>  __all__.append("swap_swapon")
>  
> 
> +_s390_dasd_format = BlockDev.s390_dasd_format
> + at override(BlockDev.s390_dasd_format)
> +def s390_dasd_format(dasd):
> +    return _s390_dasd_format(dasd)
> +__all__.append("s390_dasd_format")
> +
> +_s390_dasd_needs_format = BlockDev.s390_dasd_needs_format
> + at override(BlockDev.s390_dasd_needs_format)
> +def s390_dasd_needs_format(dasd):
> +    return _s390_dasd_needs_format(dasd)
> +__all__.append("s390_dasd_needs_format")
> +
> +_s390_dasd_online = BlockDev.s390_dasd_online
> + at override(BlockDev.s390_dasd_online)
> +def s390_dasd_online(dasd):
> +    return _s390_dasd_online(dasd)
> +__all__.append("s390_dasd_online")
> +
> +_s390_dasd_is_ldl = BlockDev.s390_dasd_is_ldl
> + at override(BlockDev.s390_dasd_is_ldl)
> +def s390_dasd_is_ldl(dasd):
> +    return _s390_dasd_is_ldl(dasd)
> +__all__.append("s390_dasd_is_ldl")
> +
> +_s390_sanitize_dev_input = BlockDev.s390_sanitize_dev_input
> + at override(BlockDev.s390_sanitize_dev_input)
> +def s390_sanitize_dev_input(dev):
> +    return _s390_sanitize_dev_input(dev)
> +__all__.append("s390_sanitize_dev_input")
> +
> +_s390_zfcp_sanitize_wwpn_input = BlockDev.s390_zfcp_sanitize_wwpn_input
> + at override(BlockDev.s390_zfcp_sanitize_wwpn_input)
> +def s390_zfcp_sanitize_wwpn_input(wwpn):
> +    return _s390_zfcp_sanitize_wwpn_input(wwpn)
> +__all__.append("s390_zfcp_sanitize_wwpn_input")
> +
> +_s390_zfcp_sanitize_lun_input = BlockDev.s390_zfcp_sanitize_lun_input
> + at override(BlockDev.s390_zfcp_sanitize_lun_input)
> +def s390_zfcp_sanitize_lun_input(lun):
> +    return _s390_zfcp_sanitize_lun_input(lun)
> +__all__.append("s390_zfcp_sanitize_lun_input")
You can drop all these overrides if they do nothing. Overrides are only
useful in cases where there are some default values for parameters or
something else Python-specific.

>  ## defined in this overrides only!
>  def plugin_specs_from_names(plugin_names):
>      ret = []
> @@ -424,6 +467,10 @@ class SwapError(BlockDevError):
>      pass
>  __all__.append("SwapError")
>  
> +class S390Error(BlockDevError):
> +    pass
> +__all__.append("S390Error")
> +
>  class UtilsError(BlockDevError):
>      pass
>  __all__.append("UtilsError")
> @@ -458,5 +505,8 @@ __all__.append("mpath")
>  swap = ErrorProxy("swap", BlockDev, [(GLib.Error, SwapError)], [not_implemented_rule])
>  __all__.append("swap")
>  
> +s390 = ErrorProxy("s390", BlockDev, [(GLib.Error, S390Error)], [not_implemented_rule])
> +__all__.append("s390")
> +
>  utils = ErrorProxy("utils", BlockDev, [(GLib.Error, UtilsError)])
>  __all__.append("utils")

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list